Consider removing DexBuilder's entry from actions with default strategies for better cache hit ratio

[jin]

Currently, DexBuilder uses the worker strategy by default as declared in BazelStrategyModule. On entirely local builds, we have seen a 10-20% build time improvement with this approach, without having the user specify additional flags.

However, this approach is overly aggressive when using RBE, or when building with remote executors in general, because the default strategy selection while using RBE's --config=remote still sets DexBuilder to use workers. This reduces the cap of the cache hit ratio.

e.g. with the minimize_downloads branch of Bazel (#6862) on the big_connected project, performing a full build after a full and a clean results in 100 uncached worker processes with all other actions (correctly) remotely cached:

$ bazel-dev build //androidAppModule0 --config=remote_android --android_aapt=aapt2 --jobs=100
INFO: Invocation ID: 181233a3-8f15-4edc-8e2b-c3830f74b993
INFO: Analysed target //androidAppModule0:androidAppModule0 (177 packages loaded, 2018 targets configured).
INFO: Found 1 target...
Target //androidAppModule0:androidAppModule0 up-to-date:
  bazel-bin/androidAppModule0/androidAppModule0_deploy.jar
  bazel-bin/androidAppModule0/androidAppModule0_unsigned.apk
  bazel-bin/androidAppModule0/androidAppModule0.apk
INFO: Elapsed time: 19.990s, Critical Path: 17.44s
INFO: 987 processes: 887 remote cache hit, 100 worker.
INFO: Build completed successfully, 1032 total actions

100 worker processes out of 987 total (89.87% cache hit), and the build took ~19.99 seconds.

If we remove the default worker strategy setting:

$ bazel-dev build //androidAppModule0 --config=remote_android --android_aapt=aapt2 --jobs=100
INFO: Invocation ID: a294c3a6-ee63-4db6-baec-f66212ea95b5
INFO: Analysed target //androidAppModule0:androidAppModule0 (177 packages loaded, 2019 targets configured).
INFO: Found 1 target...
Target //androidAppModule0:androidAppModule0 up-to-date:
  bazel-bin/androidAppModule0/androidAppModule0_deploy.jar
  bazel-bin/androidAppModule0/androidAppModule0_unsigned.apk
  bazel-bin/androidAppModule0/androidAppModule0.apk
INFO: Elapsed time: 17.637s, Critical Path: 15.69s
INFO: 987 processes: 987 remote cache hit.
INFO: Build completed successfully, 1032 total actions

100% cache hit at 17.637s. That's a 1.12x speedup.

Considering that it is simple to force a mnemonic to use a particular strategy with the --strategy flag, I propose removing DexBuilder=worker from Bazel defaults, and ask users to set it explicitly if they want to.

@kevin1e100 / @ahumesky wdyt?

@sunyal FYI

[kevin1e100]

Side question: it seems like this would apply to all defaulted-to-worker mnemonics, not just DexBuilder? Certainly seems fine to remove the default config entirely, but it seems unfortunate to do that (especially if it's done for all supported workers, see question above), since that degrades the "default" experience or requires users to set boilerplate flags. Instead it seems what you want here is to disable workers with --config=remote? Maybe another approach could be to take out the default but create a new config (e.g., --config=local or --config=workers) that we can recommend in documentation.

[jin]

--config=remote forces Javac to use remote, so that's fine.. perhaps we could add --strategy=DexBuilder=remote into RBE's config. The other affected mnemonic is Closure.

However, this would still be an issue with non-RBE users where they don't use a supplied set of flags. Calling --spawn_strategy=remote would not change DexBuilder's worker default to remote.

[jin]

On further thought, I think the best approach here is to add --strategy=DexBuilder=remote into RBE's provided configuration.

[kevin1e100]

SGTM

[agoulti]

FYI: There is ongoing work to remove the strategies that are hardcoded in the Bazel source and replace it with a more explicit list of strategies to try.

See design doc here: https://docs.google.com/document/d/10D-HGXcTRbideSyMGvn46pc4D6fuEDCzdCRgbod7tZQ/edit?usp=sharing

And more examples here: #7480

Would this serve your use case?

I will update #7480 once something is released.

[jin]

Yes! The issue here is that the worker strategy was chosen over remote, even though remote is available (which can lead to missed opportunities for cache hits). #7480 sounds like it can solve this issue.

[github-actions]

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 3 years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

[github-actions]

This issue has been automatically closed due to inactivity. If you're still interested in pursuing this, please reach out to the triage team (@bazelbuild/triage). Thanks!