-
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
Reduce Bazel binary size by deduping embedded tools #2385
Comments
I've been hacking up a prototype of option 1 in https://bazel-review.googlesource.com/#/c/8410/. Down to 99MB from 141MB. |
@damienmg , what do you think of this approach? It certainly has trade offs, but https://bazel-review.googlesource.com/#/c/8410/ reduces the bazel binary by 30%. Is this worth pursuing? |
30% is awesome, it is definitely worth it. |
option 1 is definitely preferrable. It sures has the downside of making our build a bit more hairy but meh :/ |
Another option is to not embed the files but check them into a remote workspace, and ask users to add that to their WORKSPACE file. |
Ok, I will try to clean up https://bazel-review.googlesource.com/#/c/8410/ early next week. (The one hairy part I'm unsure of the right approach is some dependencies on java_proto_library's.) Two quick observations:
|
Regarding @ulfjack comment: I think that's certainly the optimal way to go for most of these tools, especially the Android ones. In fact, some of the emulator tools that we will need for android_test we have already started the ball rolling on a new git repository for them. |
This change reduces the size taken up in the bazel binary by Android tools deploy jars from 38.2 mb to 9.8 mb, which is 15% of the bazel binary size. Also, some minor cleanups of our BUILD files. #2385 RELNOTES: None PiperOrigin-RevId: 166373241
Closing this because the android tools deduped their dependencies in e005adf. There's probably more work to be done, but ulf's comment is the better long-term plan. |
Description of the problem / feature request / question:
The Bazel binary is large, partly due to lots of duplicated dependencies in the embedded tools deploy jars.
If possible, provide a minimal example to reproduce the problem:
At 88eca6e, the compiled Bazel binary is 141M.
So we have up to 11 copies of some class files in the embedded tools. I've attached a copy of the full results of the last command: dupes.txt. One interesting thing to note is that
com.google.common
aka Guava appears 10 times.third_party/guava/guava-21.0-20161101.jar
(which is only one of the jars in//third_party:guava
) is 2.4MB. So 10 copies of that jar is ~17% of the Bazel binary.The root of the problem is that way that the
@bazel_tools
repository is set up. When we have a tool that is used by a SpawnAction in a native rule, we typically define ajava_binary
for that tool in the BUILD file and then build a deploy jar and put it in theembedded_tools
filegroup that gets put into this binary. Since most of our tools depend on common library like guava (or domain specific ones like the android common libararies), each deploy jar bundles a separate copy of it.There are a couple of solutions to this problem, each with its own drawbacks.
BUILD.tools
files. This would allow us to have only one embedded copy of guava, but has the downside that we are maintaining the dependencies twice, once inBUILD
and once inBUILD.tools
.BUILD.tools
, notBUILD
). However, it has the downside of making clean builds slower because you need to build the tools you use. It also has the downside of allowing bazel to build itself without its tools building. E.g. if I change the AndroidResourceProcessingAction, but don't update it's BUILD.tools and don't test it out,bazel build //src:bazel
will still succeed, but building an android project with the output bazel will fail.Environment info
Operating System: Ubuntu 14.04 LTS
Bazel version (output of
bazel info release
): development versionIf
bazel info release
returns "development version" or "(@non-git)", please tell us what source tree you compiled Bazel from; git commit hash is appreciated (git rev-parse HEAD
): 88eca6eThe text was updated successfully, but these errors were encountered: