Repository navigation
perf(jvm): stop merging a jvm_artifact's transitive jars into its entry - #23737
Conversation
A `jvm_artifact`'s classpath entry was one digest merging its own jar with all of its transitive jars, so a jar that many artifacts depend on, such as scala-library, was on a compile's classpath once per artifact that pulled it in, each copy under a different path. Every copy is materialized on disk, and scalac indexes each path separately and keeps that index for the life of a nailgun JVM. This keeps those jars apart: an artifact's entry holds only its own jar and lists its transitive jars' entries, which were already fetched one per jar, in a new `exports` field. Targets without sources, `resources` and Java sources with inferred exports likewise export the entries they used to merge. `with_exports`, `closure`, `args` and `immutable_inputs(_args)` include each export right after its exporter and list each path once; `args` and `immutable_inputs(_args)` take `transitive=True` to cover the closure in one walk. Because entries are now shared by many classpaths, equality visits each pair of shared entries once, like `CoarsenedTarget`'s, and the hash is cached. On a large Scala codebase, 57% of compile classpath paths were duplicates; now none are. The scalac nailgun JVMs held about 60% less heap after compiling it, and across repeated compiles their jar index grew about 40% less. Immutable inputs took 0.6 GB of disk instead of 1.9 GB. Cold compiles were slightly faster, and warm runs, compile results and test results are unchanged. `deploy_jar` no longer receives a jar twice, which changes some `duplicate_policy` outcomes; the release notes cover them.
tdyas
left a comment
tdyas
left a comment
There was a problem hiding this comment.
Very nice! I just had comments about the naming of ClasspathEntry.with_exports and ClasspathEntry.closure.
| @classmethod | ||
| def closure(cls, roots: Iterable[ClasspathEntry]) -> Iterator[ClasspathEntry]: | ||
| """All ClasspathEntries reachable from the given roots.""" | ||
| def with_exports(cls, entries: Iterable[ClasspathEntry]) -> Iterator[ClasspathEntry]: |
There was a problem hiding this comment.
The with prefix make me think that this function would have modified an attribute on an existing class, not that it yields an iterator. Maybe associate the name with iterators? Maybe iter_with_exports or iterate_with_exports?
There was a problem hiding this comment.
Agreed, renamed based on some prior naming conventions I saw with <variant>_closure
|
|
||
| visited = set() | ||
| @classmethod | ||
| def closure(cls, roots: Iterable[ClasspathEntry]) -> Iterator[ClasspathEntry]: |
There was a problem hiding this comment.
Also,. the closure name does not seem to fit with with_exports. Maybe consider giving them a common naming convention?
Yeah very fair point. 54dc46a renames the |
jvm_artifactclasspath entries held their transitive jars in a single digest. So a widely depended-on jar, like scala-library, was included in several classpath entry digests. These duplicates were then materialized to sandboxes, such as for compile. The classpath also included the duplicates. Their paths included the fingerprint of the merged digest, so caches in for example scalac held the duplicate in memory. That also grew by how many artifacts pulled in the same jars.This separates them. So a
jvm_artifactonly holds its own jar in its classpath entry digest, and a new fieldexportsis added which refers to the transitive jars by their own classpath entries. Anything that previously merged into the classpath entries now gets exported instead.export_closure,closure,argsandimmutable_inputs(_args)include each export right after its exporter, to retain the same jar order.argsandimmutable_inputs(_args)taketransitive=Trueto cover the closure in one walk. Equality uses the same trick asCoarsenedTargetandBuildGoPackageRequestto only visit each pair once, as it now represents this transitive graph instead of pre-merging it.On a relatively large scala codebase, 57% of the compile classpath was duplicates. This removes all of them. The scalac nailgun JVMs held about 60% less heap after compiling, and across repeated compiles their jar index grew about 40% less. Immutable inputs dropped from 1.9GB to 0.6GB of disk. Cold compiles were slightly faster, and cached compiles are unchanged.
A side-effect fix is that
deploy_jarno longer gets duplicates of the same jars via this mechanism, so itsduplicate_policyoutcomes behave more as expected. Before, a jar could conflict with itself just because two artifacts depended on it. That is covered in release notes.Plugins reading a classpath entry digest or filenames directly no longer see exported jars, also covered in the release notes.
Disclosure: Claude used extensively to test, verify, benchmark and iterate on implementation.