JVM build time improvements via tracking unused deps or classes [Java part]

[oliviernotteghem]

This PR is a working implementation / acts as starting point for discussing the need for the (controversial) optimization of jvm build time in Bazel, done via dynamically tracking & pruning unused dependencies, and classes (for optimal results).

Context
In some companies (including Uber), regression of local build time is a blocker for adopting Bazel for Android builds. Therefore, in-depth optimizations of incremental build present in build system like Buck are needed in order to have acceptable build time.

PR Summary

• Goal is to significantly decrease incremental (local) build times for developers, when iterating on local code changes.
• It is synergetic and built on top of (already used in several companies) transitive dependencies pruning technique (*), to provide additional wins.
• It works by ‘suppressing’ unused dep (or classes) input from the Action Cache key computation. Initial build will use all deps, but store cache entry whose key is computed off only dependencies it has used (based on jdeps output artifact, that is listing used deps/classes). Subsequent build will re-use jdeps ouput, and use it to compute cache key.
• Approach is controversial because it’s using, as input, result of previous compile action. Output is still deterministic, but it’s not a model Bazel follows usually.
• 2 modes : tracking unused deps (good), and tracking unused classes within used deps (best). Unused deps is essentially a working implementation of classpath reduction which address potential issues in large project like exports which can not be pruned statically in BUILD.bazel files, while tracking classes will achieve additional compilation avoidance by keeping track of the hash of the class ABI used (in jdeps), and rebuild only if some of these have changed since last build. In other words, ABI changes to unused classes of a used dependencies do not cause rebuild.
• it leverages existing deps usage tracking in Java and Kotlin, and extended to support more granular (classes) tracking.

Command line options

• Preferably, use on top of basic dependencies pruning (*)
• If project uses kotlin, make sure to upgrade (or patch your kotlin rules) since it's dependant on changes to kotlin rules (**)
• Use the following flags : --experimental_action_input_usage_tracker=disabled|unused_dependencies|unused_classes
• Important : When using class tracking, you need 2 additional flags to enable tracking classes for in Java and Kotlin rules respectively --experimental_track_class_usage and experimental_track_class_usage = "on" (in your kotlin toolchain definition)

Risks

• It assumes existing jdeps output generation is correct. If tracking logic is inaccurate, target that should be rebuilt might not.
• Android resources are not tracked, which means build failure may happen later (when compiling binary)
• Need additional testing / tweaking for building with ‘build without bytes’ remote cache and RBE.
• Additional hash computation for class tracking is not optimized and could slow down cold build / non-ABI builds. We haven’t seen noticeable regression though.

Benchmark
The following benchmark was done on a XL production android mobile app, forcing deep ABI-code change incremental builds.

JAVA

	Build time (secs)	# rebuilt targets
buck	44.9776914	23
bazel	498.7003218	1378
bazel w/ transitive deps pruning (*)	195.7602742	228
bazel w/ per-target compilation avoidance	38.75059199	30
bazel w/ per-class compilation avoidance	37.92243838	25

KOTLIN

	Build time (secs)	# rebuilt targets
buck	114.920902	121
bazel	383.27897	698
bazel w/ transitive deps pruning (*)	343.6441393	633
bazel w/ per-target compilation avoidance	250.5478165	341
bazel w/ per-class compilation avoidance	136.5619683	134

(*) #16426, bazelbuild/rules_kotlin#842
(**) bazelbuild/rules_kotlin#849

[sgowroji]

Hello @oliviernotteghem, Could you please resolve the branch conflicts. Thanks!

[lberki]

Before resolving the conflicts, let's discuss the general approach first; I don't want @oliviernotteghem to spend more time on this than necessary before validating whether the general idea works or not (I'm saying this without having read the source code)

[lberki]

What this change seems to do is to embed Java-specific knowledge into the generic action execution machinery of Bazel which tells which inputs files were unused in a Java compilation so that changes to those files don't cause a rebuild.

This is contrary to the direction in which we are going because we are trying to remove language-specific knowledge from Bazel. Granted, it's going to take a long time for Java due to the optimization in JavaCompileAction I'll ask about below, but still, unless the advantages are overwhelming, I'd rather not go in this direction (cc @comius )

I don't have all knowledge about Java swapped into my brain so I only have generic questions:

1. In what cases does this optimization help? My understanding is that Java compilation is first tried with a reduced classpath that mostly consists of the header jars of the direct dependencies (see JavaCompileAction#getReducedClasspath()). If BUILD files are maintained such that Java rules only depend on other rules that provide classes they directly use (which we recommend IIRC), I don't immediately see what files can be removed from the set of inputs this way.

2. Why can dependencies not be pruned statically in BUILD.bazel files? On the surface, that looks like a much better approach than working around the lack of such pruning in Bazel.

3. We already have a mechanism for removing artifacts from the inputs of an action after its execution (unused_inputs_list). What does this mechanism offer that that one doesn't?

[oliviernotteghem]

What this change seems to do is to embed Java-specific knowledge into the generic action execution machinery of Bazel which tells which inputs files were unused in a Java compilation so that changes to those files don't cause a rebuild.

Correct : the changes under /usage folder are JVM specific (Java/Kotlin), as these are the rules that currently can report, via .jdeps output artifact, used dependencies (and used classes, which is jvm-specific obviously and do not make sense for other languages/rules). However, the rest of the changes are language agnostic.

This is contrary to the direction in which we are going because we are trying to remove language-specific knowledge from Bazel. Granted, it's going to take a long time for Java due to the optimization in JavaCompileAction I'll ask about below, but still, unless the advantages are overwhelming, I'd rather not go in this direction (cc @comius )

Definitely. The code is currently encapsulated in the /usage folder to isolate logic and limit conflict when we are merging latest bazel changes. What we want is a design more consistent design with feature like discoveredInput, i.e extend existing AbstractAction class and have language-specific logic moved to corresponding rules impl. This would would make things a little less readable/easy to follow though, so I am keeping things simple for the purpose of this prototype / conversation.

I don't have all knowledge about Java swapped into my brain so I only have generic questions:

In what cases does this optimization help? My understanding is that Java compilation is first tried with a reduced classpath that mostly consists of the header jars of the direct dependencies (see JavaCompileAction#getReducedClasspath()). If BUILD files are maintained such that Java rules only depend on other rules that provide classes they directly use (which we recommend IIRC), I don't immediately see what files can be removed from the set of inputs this way.

Sure, these are good questions - happy to respond. This optimization helps for incremental builds, think of a developer making a simple code change to an XL android app. Let's say the code change is in java library XYZ. By default, Bazel will rebuild all targets that are XYZ has a transitive dependency (first scenario in benchmark), even when using ABI as compile-time jar. Why? Because the ABI jar of XYZ is an input to all these targets... This is the #1 reason why Bazel jvm build times are slow is slow (when compared to, let's say, a build system like Buck).

From there, several things can be done. Enabling classpath reduction, as you mention, can help... but is limited as it prunes only second-level dependencies (i.e deps of direct dependencies). A more efficient way is to make sure your project adheres to strict_java_deps and enable transitive dependencies pruning altogether (via #16426, bazelbuild/rules_kotlin#842). As a note, engs can use strict_java_deps=warn mode, to emit warnings that can then be parsed to automatically update BUILD.bazel file (via, typically, buildozer). This way, only direct consumer of XYZ will rebuild - this is the second scenario in benchmark. Also, existing classpath reduction impl is a no-op in this case (since there is no more transitive deps of direct dependencies as input).

However, this is not optimal nor sufficient : direct dependencies specified in BUILD.bazel might still be unecessary if they are unused, causing unnecessary rebuild if their ABI changes. Also, exported dependencies are also not handled and can create additional unnecessary invalidations. Last, tracking ABI changes at the jar level (i.e rebuilding a target whenever a dependency's ABI jar changes) causes unnecessary rebuild when the classes that changed within this dependencies are actually not used during compilation of our target. Simply put, if a target needs only class A from dependency XYZ to compile, and change to class B of XYZ should not cause target to invalidate.

I hope this answer your question about better understanding what files can be removed from the set of inputs. And to add further clarification, because any change to input set would cause invalidation, the implementation does not change or remove files from the input : it contains always 100% of the files defined at analysis time. What it does it simply ignores some of the inputs when computing the cache key and/or uses instead hashes computed from classes within ABI jar themselves (as opposed to the hash of the whole jar file).

Why can dependencies not be pruned statically in BUILD.bazel files? On the surface, that looks like a much better approach than working around the lack of such pruning in Bazel.

Exported dependencies can't be removed statically (one of the solution is to ban export entirely, which is unfortunately not as option for most companies, in order to preserve a seamless developer experience with things like dependency injection). Also, as said above, the approach of removing dependencies has limitation and doing it alone will not match Buck build system build time for example, which has the equivalent of the behavior I described above (https://github.com/facebook/buck/blob/5444652af489563ddaf595a634341dd14fd41399/src/com/facebook/buck/jvm/java/ClassUsageTracker.java)

We already have a mechanism for removing artifacts from the inputs of an action after its execution (unused_inputs_list). What does this mechanism offer that that one doesn't?

Unfortunately, this is not granular enough to implement per-class usage tracking / compilation avoidance, as this applies to action's inputs (which are dependency ABI jars in our case, not the files used by the dependency). Also, this is meant to be used rather by rules written in starlark, and, as you know, Bazel java rule is native.

I will be happy to jump in a call and help clarify some of my responses here, if this help. Don't hesitate!

[meisterT]

How much does this increase Bazel's retained heap size for your example builds? (bazel info used-heap-size-after-gc)

[lberki]

I think the conversation should be split into two threads: one about unused .jar files and the other about unused classes in used .jar files.

At Google, we, as you recommend, enable --strict_java_deps and also have extensive infrastructure to prune unused dependencies (some of the infrastructure to do so is available to the public, see in the pertinent blog entry).

Do you have a good sense of how far you could get with these two optimizations? I'd much rather have only one "officially sanctioned" way to deal with unused .jar files and the Bazel way so far seems to be strict Java deps and unused deps.

The part about unused .class files is intriguing because it offers theoretically optimal rebuilds but it also comes with a lot of risk; there are probably a number of subtle scenarios where a rebuild is required even though it's not obvious at first glance (adding or removing a class early in the classpath comes to mind, but that's very probably not the only case). It also comes at a large cost of complexity in Bazel.

@comius @cushon do you think this is a direction worth exploring at Google?

Absent the opinion of @comius and @cushon , I would be inclined to ask you to implement pruning changes to unused classes as as stateful worker instead (cc @larsrc-google ). That, AFAIU you can do without any changes to Bazel core and would provide most of the benefits you seek.

[lberki]

@alexjski also mentioned the possibility of offline analysis of .jar files: if you find that a particular set of classes in a .jar file is unused by its reverse dependencies, that's a good indication that they should be split from the .jar . Granted, this is not the theoretically best possible solution, but seems much less risky than what the change does a the moment.

[lberki]

Update: we had an accidentally non-public discussion about this pull request and the prevailing opinion seems to be that while this is very interesting work, no one is eager to take on the maintenance of such a complex piece of code and that most of the benefits can be realized by cutting dependencies in the BUILD file.

This change would also increase the heap use of Bazel and add some risk of incrementally incorrect builds, especially if we want to defend against malice; the example @alexjski brought up was when a .class is added to a .jar file early in the classpath: then it's a new class so it's not used, but at the same time overrides classes with the same name later in the classpath (I haven't checked whether this particular case is covered by your change)

So I recommend experimenting with having a java_toolchain that references a JavaBuilder instance which uses the logic here to decide whether or not to rebuild. That should be doable without changing Bazel at all and would have the advantage of not requiring the addition of domain-specific code to Bazel.

[larsrc-google]

As for the idea of letting the worker handle unused class pruning: Yes, theoretically a worker could hold a cache of "compiling this class only required these other classes" and avoid recompiling if those classes or the src itself hasn't changed. However, our experience from creating the bootclass cache (a0e5e45 etc) is not promising. The main problem is that reading the jar files is one of the most expensive steps, and we would have to do that anyway - any jar file we didn't need any classes from would have been subject to jar file pruning.

The promising part of this PR is that it works on the action cache level, so the decision to not execute the action is much earlier. I'm worried though that there are some scenarios where this would lead to incorrect compilation due to how Java works, but @cushon would be the expert on that.

[larsrc-google]

The implementation_deps attribute aims at solving the same problem for C++, it may be useful to use a common approach.

[Edited to not point at an internal doc]

[larsrc-google]

There was actually a previous attempt at doing this at Google, back in 2012. One important outcome was finding that different JDK releases can have different requirements. Between that and the manual work that would be involved in keeping the deps up to date, the project was abandoned. It's hard to say how often the JDK release breakage would happen, and automation has gotten a lot better, so these findings may not be relevant any more.

[larsrc-google]

We've been discussing this quite a bit internally. The idea is definitely of interest, expect for the reuse of exports, which is very bad for any kind of analysis tool. Implementing it with another attribute, say, api_deps, would be fine. Internally, we're doing some experiments to see how feasible it would be to use it. @oliviernotteghem, if you can update the PR to use api_deps, I would be happy to review.

[guw]

@larsrc-google Do you have a writeup or some more details about api_deps attribute design? Would it be a subset of deps or substitute for deps?

[larsrc-google]

The design as such would be as described here, except with a new attribute instead of reusing exports. The discussion has been mainly about how we can test and migrate internally, and how to keep the api_deps attribute up to date. It would definitely be a subset of deps, as deps is still required for building the binaries/running the tests. See our comments above for critical issues.

[guw]

Thank you! I think I am confused because I can not find directly in the PR files where exports is being used.

[larsrc-google]

Yeah, that's a bit confusing too, but as mentioned above there are worse issues with using exports.

[guw]

@lberki Not sure if your questions were all answered but I'd like to give a perspective why this is important to us.

I don't have all knowledge about Java swapped into my brain so I only have generic questions:

1. In what cases does this optimization help? My understanding is that Java compilation is first tried with a reduced classpath that mostly consists of the header jars of the direct dependencies (see JavaCompileAction#getReducedClasspath()). If BUILD files are maintained such that Java rules only depend on other rules that provide classes they directly use (which we recommend IIRC), I don't immediately see what files can be removed from the set of inputs this way.

The problem that is biting us heavily is that all transitive jars (not just the direct deps declared in build files) are part of the cache key of JavaCompileAction.

bazel/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java

Lines 233 to 235 in 70b43db

	// As the classpath is no longer part of commandLines implicitly, we need to explicitly add
	// the transitive inputs to the key here.
	actionKeyContext.addNestedSetToFingerprint(fp, transitiveInputs);

Given the following (simple) graph:

java_library( name = "A" )
java_library( name = "B", deps = ["A"])
java_library( name = "C", deps = ["B"])

Given the following change:

• changing a method implementation within A, i.e. no change to the ijar of A

Expected outcome:

• B and C not rebuild

Actual outcome:

• B and C rebuild

The reduced classpath mode is a weird one. Even though it reduced the number of transitive jars, this only helps in combination with remote/disk_cache and a stable reduction. However, the reduction isn't stable. For example, the implementation change can result in an unused direct deps, which would be reduced and you end up rebuilding B and C unnecessarily again.

2. Why can dependencies not be pruned statically in BUILD.bazel files? On the surface, that looks like a much better approach than working around the lack of such pruning in Bazel.

I think in general pruning direct deps should really happen with a tool via BUILD.bazel files. I do think that this PR as well a #16426 are trying to solve the same problem, i.e. unnecessary rebuilds. This brings in the transitive deps, which cannot be pruned statically.

3. We already have a mechanism for removing artifacts from the inputs of an action after its execution (unused_inputs_list). What does this mechanism offer that that one doesn't?

I don't see this supported currently with JavaCompileAction. However, if there is interest I am willing to look into allowing a Java toolchain to respond to Bazel with such an unused list (we have one based on the Eclipse Java compiler). I can probably make this configurable. But I think this requires some significant updates to JavaCompileAction to make that work. I need to know if that is something The Bazel team is willing to consider.

Right now we are seeing a lot unnecessary rebuilds, which result into a 7-10 minutes time waste for some developers working in some areas of our graph. Thus, we really like to do something but we need to know which approach The Bazel team would be willing to merge.

[kevin1e100]

Given the following change:

changing a method implementation within A, i.e. no change to the ijar of A
Expected outcome:

B and C not rebuild
Actual outcome:

B and C rebuild

That... would seem like a bug. However, it occurs to me that B and C might still be rebuilt if A's jdeps file changes. Is that's what's happening? Maybe just file a separate bug with a repro we can look at.

The reduced classpath mode is a weird one. Even though it reduced the number of transitive jars, this only helps in combination #16426 (comment) and a stable reduction.

I guess I'm mildly surprised that the reduced classpath optimization would only work with a remote cache. If that's the case, I'd file an FR to see if that restriction can be removed.

However, the reduction isn't stable. For example, the implementation change can result in an unused direct deps, which would be reduced and you end up rebuilding B and C unnecessarily again.

This is because the .jdeps inputs change, right? If reduced classpath mode isn't working anyway, you might consider disabling it, which I'd hope would avoid passing .jdeps files to JavaBuilder and at least avoid this scenario.

I don't see this supported currently with JavaCompileAction. However, if there is interest I am willing to look into allowing a Java toolchain to respond to Bazel with such an unused list

The unused_inputs file would basically contain the inverse of the .jdeps file, so I feel supporting it should be eminently feasible. How effective it would be I'm not sure: I suspect it wouldn't address any of the issues mentioned above where direct inputs did change. But it should be helpful in scenarios with longer chains of dependencies.

[lberki]

@lberki Not sure if your questions were all answered but I'd like to give a perspective why this is important to us.

I don't have all knowledge about Java swapped into my brain so I only have generic questions:

1. In what cases does this optimization help? My understanding is that Java compilation is first tried with a reduced classpath that mostly consists of the header jars of the direct dependencies (see JavaCompileAction#getReducedClasspath()). If BUILD files are maintained such that Java rules only depend on other rules that provide classes they directly use (which we recommend IIRC), I don't immediately see what files can be removed from the set of inputs this way.

The problem that is biting us heavily is that all transitive jars (not just the direct deps declared in build files) are part of the cache key of JavaCompileAction.

bazel/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java

Lines 233 to 235 in 70b43db

	// As the classpath is no longer part of commandLines implicitly, we need to explicitly add
	// the transitive inputs to the key here.
	actionKeyContext.addNestedSetToFingerprint(fp, transitiveInputs);

Given the following (simple) graph:

java_library( name = "A" )
java_library( name = "B", deps = ["A"])
java_library( name = "C", deps = ["B"])

Given the following change:

• changing a method implementation within A, i.e. no change to the ijar of A

Expected outcome:

• B and C not rebuild

Actual outcome:

• B and C rebuild

Can you give a (preferably minimal) reproduction for this? I'm with @kevin1e100 in that this looks like a bug.

The reduced classpath mode is a weird one. Even though it reduced the number of transitive jars, this only helps in combination with remote/disk_cache and a stable reduction. However, the reduction isn't stable. For example, the implementation change can result in an unused direct deps, which would be reduced and you end up rebuilding B and C unnecessarily again.

I'm afraid I don't understand this; you're right that this only works with --disk_cache / --remote_cache, but I don't get the "implementation change can result in an unused direct dep" part.

2. Why can dependencies not be pruned statically in BUILD.bazel files? On the surface, that looks like a much better approach than working around the lack of such pruning in Bazel.

I think in general pruning direct deps should really happen with a tool via BUILD.bazel files. I do think that this PR as well a #16426 are trying to solve the same problem, i.e. unnecessary rebuilds. This brings in the transitive deps, which cannot be pruned statically.

3. We already have a mechanism for removing artifacts from the inputs of an action after its execution (unused_inputs_list). What does this mechanism offer that that one doesn't?

I don't see this supported currently with JavaCompileAction. However, if there is interest I am willing to look into allowing a Java toolchain to respond to Bazel with such an unused list (we have one based on the Eclipse Java compiler). I can probably make this configurable. But I think this requires some significant updates to JavaCompileAction to make that work. I need to know if that is something The Bazel team is willing to consider.

I'll defer to @hvadehra on this. I'm not against adding some sort functionality that allows the Java toolchain to signal to Bazel that some inputs were not required. In fact, it might even be that you don't need to do anything extra other than changing JavaCompileAction to signal to Bazel that some inputs were not needed, akin to how CppCompileAction does the same with unnecessary .h files because the .jdeps file already contains the list of .jar files that were actually used.

Right now we are seeing a lot unnecessary rebuilds, which result into a 7-10 minutes time waste for some developers working in some areas of our graph. Thus, we really like to do something but we need to know which approach The Bazel team would be willing to merge.

[fmeum]

Since I got pretty confused about what works and what doesn't, I created a small test bed for Bazel Java compilation avoidance here, based on a very recent HEAD commit.

If you run ./script.sh, you will find that Bazel with --experimental_java_classpath=bazel and a disk cache avoids all unnecessary recompilations except for the case of a signature change in an unused direct dep. The latter could potentially be improved, which I assume is what this PR plans to do.

@guw I can't reproduce the rebuilds caused by implementation-only changes with this. It would be very helpful if you could try out the reproducer and add a case for the situations in which you would like Bazel to avoid more rebuilds, in particular one in which the reduction is "unstable".

[guw]

Thanks to @fmeum for the repo we now have a simple reproducible scenario with shows unnecessary compilation and/or cache misses.

fmeum/bazel-java-compilation-avoidance#2

[fmeum]

The cases we have identified so far in which compilation could be avoided if Bazel was "smarter" so far fall into the following categories:

1. Changes to unused explicit direct deps: If //:foo has //:bar in deps but doesn't use it, then signature changes to //:bar trigger a rebuild of //:foo. This could be avoided by removing the unused dep via static tooling (unused_deps) or by using the equivalent of Starlark's unused_inputs_list to remove the jars of //:bar from //:foo's local action key. Downside of the latter is the increased complexity as well as not being able to share this cache hit with other users - unused_inputs_list only prunes files from the action key, not the remote cache entry.
2. Changes to unused implicit direct deps: If //:foo has //:bar in deps and //:bar has //:quz in exports but //:foo doesn't use //:quz, then signature changes to //:quz trigger a rebuild of //:foo. This is similar to 1., it's just more difficult to resolve with static tooling without intelligently breaking up targets (which may not be possible). This is a use case that may only be solvable via input pruning but is also only relevant for codebases that have large exports.
3. Builds with --nojava_header_compilation: As @guw mentioned on Unnecessary compile with private-B-change-removes-B-to-A-dep.patch fmeum/bazel-java-compilation-avoidance#2, they disable Java header compilation as it increases build times for them. But, as is easy to check on the reproducer, Java header compilation appears to prevent the unnecessary rebuild in this case.
4. Large deps which contain both used and unused classes: This can only be improved automatically by tracking per-class usage information, which is both very invasive and may, at least if not implemented correctly, lead to incremental incorrectness in edge cases (e.g. classpath order).

(If anyone can identify any other situation, please speak up :-))

Given the above, I would suggest the following plan:

1. Speed up Turbine in Bazel by either making it a worker or a native image. Just replacing the jar with a native image with no special optimizations applied (I know very little about GraalVM ;-)) reduces the time spent header compiling everything in the transitive clojure of //src:bazel-dev from 5m22s down to 1m9s on my laptop, a speedup of more than 4.5x.
2. Provide external rulesets with the ability to do what --experimental_java_classpath=bazel does for Java. It would be interesting to learn what the Google-internal Kotlin rules do here. Also, think of ways to make --experimental_java_classpath=bazel the default in Bazel and thus more widely known and used.
3. If 1. and 2. by themselves aren't sufficient and static tooling doesn't work well enough for Bazel, consider adding a flag to Bazel that optionally applies input pruning to Java compile actions. https://github.com/bazel-contrib/unused-jvm-deps doesn't handle external dependencies out of the box, so it's important to understand that the unused deps tooling works much better inside Google than outside.
4. If neither of that suffices to bring down compilation times as much as needed, discuss more invasive changes such as per-class tracking.

@cushon @hvadehra for their feedback on 1. and 2. in particular.

[fmeum]

I drafted a PR to get started on the "native turbine image" step of this plan: #19361