eth: make duktape dependency optional #20590
Comments
|
To further drive why the 25s sequential build time is painful: the project in question builds from scratch in a total of 40s or so, including link time. Given that the Go linker is going to get significantly faster soon, I assume the last chunk of linker work will get smaller. This will leave us with duktape being to blame for why over half of the build time is only using one CPU, being a bottleneck for the overall build time. |
|
Duktape isn't necessary for your average Ethereum node, however it is used by transaction tracing. The reason we went with it is to allow anyone to upload their own .js tracing script instead of relying on us to build and maintain all the possible tracers in Go ourselves. And yes, you hit the nail on the head, it's used due to performance reasons (an order of magnitude faster than Otto). I don't think we can nuke it out altogether, but I think adding an optional build tag to disable it could be easy enough if someone knows what they are doing. Will look into how much pain it would be to disable tracing via a tag. |
|
Sounds good - thanks for considering this :) |
|
Another drive-by thought - would it be possible to keep it entirely out of the dependency tree of the eth package? For example, if a user needs transaction tracing, they can explicitly underscore-import a package that pulls in duktape and enables that feature. Or, perhaps, have the default be pure Go (Otto?) and use an underscore import to switch to duktape for performance. |
|
The fundamental issue here is that package eth depends on eth/tracers, the only reason being that the To remove the dependency, the path requiring the least amount of changes is refactoring eth/tracers into its own |
|
I personally think that would be great. In my opinion, heavy CGo dependencies such as duktape should be opt-in. That is, one should have to explicitly add it to the import graph. |
|
For what it's worth, we ended up with a temporary workaround which was to do a Still, that's only a hacky solution that works when we do local builds, so a more general solution for any Go package would be better, such as making the import opt-in. |
|
In general this would be great to have. |
|
I think this is fixed: |
Since ethereum/go-ethereum#20590 was fixed last year, the core go-ethereum package no longer pulls in go-duktape. We can see this because neither our go.mod nor go.sum mention duktape.
Since ethereum/go-ethereum#20590 was fixed last year, the core go-ethereum package no longer pulls in go-duktape. We can see this because neither our go.mod nor go.sum mention duktape.
Apologies for skipping the issue template, but this isn't a bug with go-ethereum per se.
Depending on
github.com/ethereum/go-ethereum/ethis pretty basic. Unfortunately, it pulls in a pretty heavy cgo dependency:For some quick numbers - most Go packages in a regularly sized project build in under two seconds on my laptop, and they mostly build in parallel with other packages. However,
go-duktapetakes a whopping 25s to build on the same laptop, supposedly because it has a lot of C code to build without any parallelism.We could make this faster in the Go toolchain, by building cgo files in parallel (golang/go#9887). However, I doubt that would help much. The vast majority of the duktape codebase is in a single C file clocking in at almost four megabytes: https://github.com/olebedev/go-duktape/blob/v3/duktape.c
The bottom line is that duktape is a heavy cgo dependency, and there currently isn't a way around it. Is it truly a core part of go-ethereum? Assuming that it is not, could it be removed from the dependency tree when one simply imports the "core" packages?
If it is absolutely necessary for the core packages, I think you still have other options:
duktape, similar tomath/big's-tags=math_big_pure_gogo-ethereumwithCGO_ENABLED=0, which currently fails due to duktape not buildingThe text was updated successfully, but these errors were encountered: