Speed up build by avoiding build config duplication #753
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fhanau
force-pushed
the
felix/bazel-cfg-speedup
branch
2 times, most recently
from
ac29e67 to
f048183
Compare
ohodson
reviewed
Jun 12, 2023
fhanau
changed the title
fhanau
force-pushed
the
felix/bazel-cfg-speedup
branch
6 times, most recently
from
80aef6d to
feed276
Compare
ohodson
reviewed
Jun 15, 2023
patches/v8/0011-Use-target-cfg.patch
Outdated
| + }, | ||
| +) | ||
| -- | ||
| 2.39.2 (Apple Git-143) |
There was a problem hiding this comment.
JFYI, the v8-updates.md doc suggests using "git format-patch --full-index -k --no-signature".
ohodson
approved these changes
Jun 15, 2023
There was a problem hiding this comment.
Thanks Felix, I feel I've missed this PR transitioning from draft to ready so apologies if you've been held up.
From experience of updating V8 in workerd, the top-level BUILD.bazel is one of the places that takes time to deal with. So additional changes there come with some mild concerns, but we'll figure it out.
Compile substantially all of workerd in the bazel 'target' configuration instead of 'exec', which allows us to avoid compiling major components twice. This should offer a large performance improvement, especially when rebuilding after a V8 update or building all targets and drive down the worst-case CI build time significantly. Bazel supports an exec configuration for building code that needs to run on the host machine during the build, e.g. tools used to generate source files or type definitions. When not cross-compiling exec and target will be the same, but bazel does not offer a flag to use just one configuration when the configurations are identical (bazelbuild/bazel#14848), making manual changes necessary. Bazel makes it very difficult to set the config for a target at build time, so default to using cfg "target" globally for now.
fhanau
force-pushed
the
felix/bazel-cfg-speedup
branch
from
feed276 to
e4dda9a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Compile substantially all of workerd in the bazel 'target' configuration
instead of 'exec', which allows us to avoid compiling major components
twice.
This should offer a large performance improvement, especially when
rebuilding after a V8 update or building all targets and drive down the
worst-case CI build time significantly.
Bazel supports an exec configuration for building code that needs to run
on the host machine during the build, e.g. tools used to generate source
files or type definitions. When not cross-compiling exec and target will
be the same, but bazel does not offer a flag to use just one configuration
when the configurations are identical (bazelbuild/bazel#14848),
making manual changes necessary.
Bazel appears to offer no way to detect if exec and host are the same, so
this feature is implemented through an opt-in command line flag enabled in
.bazelrc, which makes it possible to easily disable the change when
cross-compilation support is needed.
Some of the V8 changes are extensive; fortunately they largely affect bazel code and not the list of source files, which is expected to change infrequently. I will try to upstream at least the torque-related changes.
Note: This does not affect the upstream build, no upstream PR is needed.