-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Multiplatform output paths are safe, correct, and efficient #6526
Comments
2018 EOY update: See #6527 (comment). More updates coming Q1'2019. |
April '19 update: Detailed plans at Experimental Content-Based Output Paths (please comment!). Goal is to get an |
P1 issue review: still relevant, still very much intend to explore this but I simply am unable to put time into it at the moment. Hoping I can pick this up next quarter. |
This is being up-prioritized with about ~1 dev's full-time commitment over the next 3 months. |
Is there an escalation path at Google (i.e., someone we could reach out to) that could help align on business priorities? |
Write to me as a technical contact (gregce@bazel.build) explaining your needs as best you can. I'm happy to chat technical concerns and CC in folks who can help with business priorities. |
To elaborate on #6526 (comment), I believe the generic solution described in this issue and #6526 (comment) is nuanced enough that it'd have to go through a long experimental phase before we could consider productionizing parts. I'd still like to get to that phase because it would still let interested folks opt in, explore, and help evolve its path. But we're also trying to explore if there are more limited variations we could hack out more quickly while avoiding the deeper design issues. That's going to be the focus of the current up-prioritization. We have an idea of something (hopefully) quick and dirty that could approximate a lot of this, probably with a small code injection into the remote executor client. I'll continue to follow up here. Speaking of, is anyone interested in this and not using remote execution? |
We are. We're in a setup where we have macOS dev machines and Linux CI machines for Android builds. We're hoping to use remote exec at some point but atm we're only using a remote cache, which we're thinking this might help with since right now the 2 platforms don't share cache hits |
Acknowledged, thanks. |
Exact same with @keith for my team's use case except plain old Java->jars, not Android. |
I'm sorry I haven't updated this for a while. Quick update is I recently experimented with a limited form of this as suggested at #6526 (comment). Initial results look promising. I want to do another test over a sample project (maybe Bazel itself?) to verify the results. Then I need to look at injection points, since Bazel has different APIs for delegating to local and remote executors and this change is likely to live in the implementation layer. I'm spending a good chunk of this week doing the above. As always, please ping (or reach out to me directly) if you're wondering what's up in between updates. |
Thanks for update @gregestren! Your work in this area is very much appreciated! Would you like to elaborate about the scope of the “limited form" of #6526? Which use cases do you expect it to support, and which not? I interpret it as that a complete and generic solution with production quality of #6526, is still the final goal, but realistically more than a year away. Is that correctly interpreted? Would you dare to make a very rough time estimate? Again, thank you for all effort in this area, it is very important for us to not explode the executor workload when using transitions for our c/c++ applications, in examples like: https://groups.google.com/g/bazel-discuss/c/zVEc7gzbyu0 |
@ulrfa sure! The generic approach I outlined in #6526 (comment) tries to balance a variety of needs, including the need that the paths the executor sees are identical to what appears in Bazel's final output tree. That makes actions that write manifests or debug symbol paths safe. If we drop that requirement, that opens up a much simpler algorithm: strip the config-specific info completely from the paths before shipping them to the executor, then add them back when writing them to Bazel's output tree. So That exposes the risks from my first paragraph. But not every action has that risk. Lots of actions truly don't care what their input or output paths look like. So this new approach would introduce criteria for which actions are "safe" in this regard and rewrite paths for safe actions. We could presumably start with a small and conservative safety set, then expand as we vet more actions. Java actions I think are particularly good candidates for this. C++ has the extra challenge of debug mode symbol paths. But that's only a certain subset of C++ actions. Not all of them. For https://groups.google.com/g/bazel-discuss/c/zVEc7gzbyu0, another complementary idea is "trimming" - if it's really only the binary that consumes the flag, we could simply remove that flag from configurations in its dependencies. I already have a tool we could conceptually use to make this happen. But it'd require preprocessing: every time a BUILD file changes you'd have to rerun that tool to annotate the BUILD rules. A 100% automatic approach would be ideal. Time-wise, I'd like to share some clearer experimental results on some Java actions over the next month or two. If that all looks good I don't see why we can't enable this limited approach by, say, January. It might take more tweaks to figure out the C++ nuances. |
Thanks @gregestren!
What subset of C++ actions do you mean? Does the subset include all actions compiling source code with debug symbol paths? Unfortunately we need to compile our C/C++ code with debug symbol paths.
Trimming is interesting! I guess that would also reduce build graph size and RAM requirement. I will have a look at your tool! But unfortunately, we have a deep build graph, with many configuration options consumed by lots of cc_library. It would be hard for us without an automatic approach. Do you as final goal, aim for an automatic trimming solution and/or an output path solution handling C/C++ code with debug symbol paths? If yes, would you like to give a rough time estimate? I'm sorry to bother you about the time estimates. We are considering if going all-in with transitions, and your input about what to expect, and roughly when, is essential for us in that decision. |
Yes, I mean actions that rely on paths for resolving debug symbols vs. those that don't. Although it's not just that, it's also whatever consumes those paths (like This isn't to say there aren't options. We could conceivably rewrite the symbol paths after the fact. But that'd be a specialized effort.
They key point in my mind is if your top-level binary is the only one that actually consumes the flag in question, then we'd have some real options, no matter what
That would be wonderful, but it's an ambitious goal that I can't credibly put a timeline on. I'm trying to focus effort on incremental steps forward, so we can see credible practical progress vs. a reallllly long wait with unclear outcome. So in my view the status quo is for us to identify optimizable use cases and try to optimize them. Not try to automatically make everything work at peak efficiency.
No worries. I'm not sure my input is helping you with this decision. I guess I'm ultimately saying we need to understand the precise requirements of specific builds and aim optimizations at improving those builds (and whatever other builds have the same patterns). So the real answer, as usual, is in the details. |
Hi! I try here to describe our use case: We build C/C++ applications for an embedded system with quite large build graphs with very many configurable options using "User-defined build settings" https://docs.bazel.build/versions/master/skylark/config.html Examples of configurable options are:
The targets that are affected by the options can be at any level in the dependency tree. The typical use case is that you build one application with some specified configurable options. If you do this on the command line everything will be built in the default output tree. If you do this in a transition, nothing can be reused between the builds. This will cause a lot of rebuilds if something in e.g. some common code is changed. It will also force the need of a much larger remote cache storage. We need to be able to debug the application targets, we use gdb and depend on that the debug symbols are correct to be able to show the source files. |
Getting rid of the single boolean field on `AbstractFileWriteAction` saves 8 bytes on each subclass instance due to padding. Also use a lambda to define `newDeterministicWriter` if possible for improved readability. This prepares for future changes that will add fields to `CppModuleMapAction` to support path mapping. Work towards bazelbuild#6526
Getting rid of the single boolean field on `AbstractFileWriteAction` saves 8 bytes on each subclass instance due to padding. Also use a lambda to define `newDeterministicWriter` if possible for improved readability. This prepares for future changes that will add fields to `CppModuleMapAction` to support path mapping. Work towards bazelbuild#6526
Getting rid of the single boolean field on `AbstractFileWriteAction` saves 8 bytes on each subclass instance due to padding. Also use a lambda to define `newDeterministicWriter` if possible for improved readability. This prepares for future changes that will add fields to `CppModuleMapAction` to support path mapping. Work towards bazelbuild#6526
Getting rid of the single boolean field on `AbstractFileWriteAction` saves 8 bytes on each subclass instance due to padding. Also use a lambda to define `newDeterministicWriter` if possible for improved readability. This prepares for future changes that will add fields to `CppModuleMapAction` to support path mapping. Work towards bazelbuild#6526
Getting rid of the single boolean field on `AbstractFileWriteAction` saves 8 bytes on each subclass instance due to padding. Also use a lambda to define `newDeterministicWriter` if possible for improved readability. This prepares for future changes that will add fields to `CppModuleMapAction` to support path mapping. Work towards bazelbuild#6526
Getting rid of the single boolean field on `AbstractFileWriteAction` saves 8 bytes on each subclass instance due to padding. Also use a lambda to define `newDeterministicWriter` if possible for improved readability. This prepares for future changes that will add fields to `CppModuleMapAction` to support path mapping. Work towards bazelbuild#6526
I just opened a discussion that documents the current state of path mapping in Bazel: #22658 |
Getting rid of the single boolean field on `AbstractFileWriteAction` reduces padding on each subclass instance and in particular frees up a 4-byte field on `CppModuleMapAction`. Also use a lambda to define `newDeterministicWriter` if possible for improved readability. This prepares for future changes that will add fields to `CppModuleMapAction` to support path mapping. Work towards #6526 Closes #22609. PiperOrigin-RevId: 643340715 Change-Id: Id3af26049098e6dfa731f0e7a1be6709bea0d9f2
Getting rid of the single boolean field on `AbstractFileWriteAction` reduces padding on each subclass instance and in particular frees up a 4-byte field on `CppModuleMapAction`. Also use a lambda to define `newDeterministicWriter` if possible for improved readability. This prepares for future changes that will add fields to `CppModuleMapAction` to support path mapping. Work towards bazelbuild#6526 Closes bazelbuild#22609. PiperOrigin-RevId: 643340715 Change-Id: Id3af26049098e6dfa731f0e7a1be6709bea0d9f2
Getting rid of the single boolean field on `AbstractFileWriteAction` reduces padding on each subclass instance and in particular frees up a 4-byte field on `CppModuleMapAction`. Also use a lambda to define `newDeterministicWriter` if possible for improved readability. This prepares for future changes that will add fields to `CppModuleMapAction` to support path mapping. Work towards bazelbuild#6526 Closes bazelbuild#22609. PiperOrigin-RevId: 643340715 Change-Id: Id3af26049098e6dfa731f0e7a1be6709bea0d9f2
…classes (#22845) Getting rid of the single boolean field on `AbstractFileWriteAction` reduces padding on each subclass instance and in particular frees up a 4-byte field on `CppModuleMapAction`. Also use a lambda to define `newDeterministicWriter` if possible for improved readability. This prepares for future changes that will add fields to `CppModuleMapAction` to support path mapping. Work towards #6526 Closes #22609. PiperOrigin-RevId: 643340715 Change-Id: Id3af26049098e6dfa731f0e7a1be6709bea0d9f2 Closes #22749 Closes #22750
Basic support for path mapping for `CppCompile` actions is added by wiring up `PathMapper` with: * structured path variables for files and include paths (`Artifact` and `NestedSet<PathFragment>`) * inputs/outputs via `Spawn#getPathMapper()` * header discovery Also turns `external_include_paths` into a structured variable, which was missed in #22463. The following features are known to be unsupported for now: * `layering_check` (requires rewriting paths to and in module maps) * source tree artifacts (requires wiring up `PathMapper` in `CppCompileActionTemplate`) * location expanded paths to generated files such as sanitizer suppression lists (requires heuristically rewriting paths in `user_compile_flags`) These limitations will be lifted in follow-up PRs. Work towards #6526 RELNOTES: Experimental support for path mapping `CppCompile` actions can be enabled via `--modify_execution_info=CppCompile=+supports-path-mapping`. Closes #22445. PiperOrigin-RevId: 646109274 Change-Id: I6f4eb92b6be3052547f144c681b6588e9fc40693
Basic support for path mapping for `CppCompile` actions is added by wiring up `PathMapper` with: * structured path variables for files and include paths (`Artifact` and `NestedSet<PathFragment>`) * inputs/outputs via `Spawn#getPathMapper()` * header discovery Also turns `external_include_paths` into a structured variable, which was missed in bazelbuild#22463. The following features are known to be unsupported for now: * `layering_check` (requires rewriting paths to and in module maps) * source tree artifacts (requires wiring up `PathMapper` in `CppCompileActionTemplate`) * location expanded paths to generated files such as sanitizer suppression lists (requires heuristically rewriting paths in `user_compile_flags`) These limitations will be lifted in follow-up PRs. Work towards bazelbuild#6526 RELNOTES: Experimental support for path mapping `CppCompile` actions can be enabled via `--modify_execution_info=CppCompile=+supports-path-mapping`. Closes bazelbuild#22445. PiperOrigin-RevId: 646109274 Change-Id: I6f4eb92b6be3052547f144c681b6588e9fc40693
Basic support for path mapping for `CppCompile` actions is added by wiring up `PathMapper` with: * structured path variables for files and include paths (`Artifact` and `NestedSet<PathFragment>`) * inputs/outputs via `Spawn#getPathMapper()` * header discovery Also turns `external_include_paths` into a structured variable, which was missed in bazelbuild#22463. The following features are known to be unsupported for now: * `layering_check` (requires rewriting paths to and in module maps) * source tree artifacts (requires wiring up `PathMapper` in `CppCompileActionTemplate`) * location expanded paths to generated files such as sanitizer suppression lists (requires heuristically rewriting paths in `user_compile_flags`) These limitations will be lifted in follow-up PRs. Work towards bazelbuild#6526 RELNOTES: Experimental support for path mapping `CppCompile` actions can be enabled via `--modify_execution_info=CppCompile=+supports-path-mapping`. Closes bazelbuild#22445. PiperOrigin-RevId: 646109274 Change-Id: I6f4eb92b6be3052547f144c681b6588e9fc40693
Basic support for path mapping for `CppCompile` actions is added by wiring up `PathMapper` with: * structured path variables for files and include paths (`Artifact` and `NestedSet<PathFragment>`) * inputs/outputs via `Spawn#getPathMapper()` * header discovery Also turns `external_include_paths` into a structured variable, which was missed in bazelbuild#22463. The following features are known to be unsupported for now: * `layering_check` (requires rewriting paths to and in module maps) * source tree artifacts (requires wiring up `PathMapper` in `CppCompileActionTemplate`) * location expanded paths to generated files such as sanitizer suppression lists (requires heuristically rewriting paths in `user_compile_flags`) These limitations will be lifted in follow-up PRs. Work towards bazelbuild#6526 RELNOTES: Experimental support for path mapping `CppCompile` actions can be enabled via `--modify_execution_info=CppCompile=+supports-path-mapping`. Closes bazelbuild#22445. PiperOrigin-RevId: 646109274 Change-Id: I6f4eb92b6be3052547f144c681b6588e9fc40693
Basic support for path mapping for `CppCompile` actions is added by wiring up `PathMapper` with: * structured path variables for files and include paths (`Artifact` and `NestedSet<PathFragment>`) * inputs/outputs via `Spawn#getPathMapper()` * header discovery Also turns `external_include_paths` into a structured variable, which was missed in bazelbuild#22463. The following features are known to be unsupported for now: * `layering_check` (requires rewriting paths to and in module maps) * source tree artifacts (requires wiring up `PathMapper` in `CppCompileActionTemplate`) * location expanded paths to generated files such as sanitizer suppression lists (requires heuristically rewriting paths in `user_compile_flags`) These limitations will be lifted in follow-up PRs. Work towards bazelbuild#6526 RELNOTES: Experimental support for path mapping `CppCompile` actions can be enabled via `--modify_execution_info=CppCompile=+supports-path-mapping`. Closes bazelbuild#22445. PiperOrigin-RevId: 646109274 Change-Id: I6f4eb92b6be3052547f144c681b6588e9fc40693
Basic support for path mapping for `CppCompile` actions is added by wiring up `PathMapper` with: * structured path variables for files and include paths (`Artifact` and `NestedSet<PathFragment>`) * inputs/outputs via `Spawn#getPathMapper()` * header discovery Also turns `external_include_paths` into a structured variable, which was missed in #22463. The following features are known to be unsupported for now: * `layering_check` (requires rewriting paths to and in module maps) * source tree artifacts (requires wiring up `PathMapper` in `CppCompileActionTemplate`) * location expanded paths to generated files such as sanitizer suppression lists (requires heuristically rewriting paths in `user_compile_flags`) These limitations will be lifted in follow-up PRs. Work towards #6526 RELNOTES: Experimental support for path mapping `CppCompile` actions can be enabled via `--modify_execution_info=CppCompile=+supports-path-mapping`. Closes #22445. PiperOrigin-RevId: 646109274 Change-Id: I6f4eb92b6be3052547f144c681b6588e9fc40693 Closes #22875 Co-authored-by: Yun Peng <pcloudy@google.com>
In `CommandLines`, the very first argument of the first command line is always a path to an executable. As such, it should be path mapped, even when it is a string. This wasn't the case for `SpawnAction`'s created via `ctx.actions.run(executable = <some string>)`. Work towards #6526 Work towards #22366 Closes #22844. PiperOrigin-RevId: 656258007 Change-Id: Ia046a7cc66aae51aec764e2f1c49e1d4f69e4b37
In `CommandLines`, the very first argument of the first command line is always a path to an executable. As such, it should be path mapped, even when it is a string. This wasn't the case for `SpawnAction`'s created via `ctx.actions.run(executable = <some string>)`. Work towards bazelbuild#6526 Work towards bazelbuild#22366 Closes bazelbuild#22844. PiperOrigin-RevId: 656258007 Change-Id: Ia046a7cc66aae51aec764e2f1c49e1d4f69e4b37 Closes bazelbuild#23040
In `CommandLines`, the very first argument of the first command line is always a path to an executable. As such, it should be path mapped, even when it is a string. This wasn't the case for `SpawnAction`'s created via `ctx.actions.run(executable = <some string>)`. Work towards bazelbuild#6526 Work towards bazelbuild#22366 Closes bazelbuild#22844. PiperOrigin-RevId: 656258007 Change-Id: Ia046a7cc66aae51aec764e2f1c49e1d4f69e4b37 Closes bazelbuild#23040
In `CommandLines`, the very first argument of the first command line is always a path to an executable. As such, it should be path mapped, even when it is a string. This wasn't the case for `SpawnAction`'s created via `ctx.actions.run(executable = <some string>)`. Work towards #6526 Work towards #22366 Closes #22844. PiperOrigin-RevId: 656258007 Change-Id: Ia046a7cc66aae51aec764e2f1c49e1d4f69e4b37 Closes #23040
C++ action templates are now properly path mapped. Since the default Unix toolchain doesn't support object file groups on macOS, `apple_support` is needed in the test. This requires wiring up `LocalEnvProvider` in the remote worker to get `DEVELOPER_DIR` to be set, which in turn requires improving the runfiles handling for the the worker. Work towards #6526 Closes #22890. PiperOrigin-RevId: 657733074 Change-Id: I132e338d7a44964a8a1aad3062a5c9a4c8e79e57
C++ action templates are now properly path mapped. Since the default Unix toolchain doesn't support object file groups on macOS, `apple_support` is needed in the test. This requires wiring up `LocalEnvProvider` in the remote worker to get `DEVELOPER_DIR` to be set, which in turn requires improving the runfiles handling for the the worker. Work towards bazelbuild#6526 Closes bazelbuild#22890. PiperOrigin-RevId: 657733074 Change-Id: I132e338d7a44964a8a1aad3062a5c9a4c8e79e57
Tracking issue on Bazel Configurability Roadmap
By "multiplatform" I mean any scenario where two different rules in the same build build with different settings. This also includes non-platform settings like app version, but "multiplatform" is a concise term to capture the essence.
Long-story short is
bazel-out/$(cpu)-$compilation_mode)/...
doesn't work well for multiplatform builds:cpu
is redundant for cpu-agnostic actions (efficiency issue: switching up the CPU shouldn't require re-executing these actions: see Java compilation doesn't include cpu in output paths #6527)This issue tracks the long and complicated effort of making a better output path syntax. Expect the next deliverable on this to be a design doc.
The text was updated successfully, but these errors were encountered: