[2.x] Intern analysis values while deserializing - #1754
Merged
Conversation
hoangmaihuy
force-pushed
the
perf/analysis-intern
branch
from
July 25, 2026 06:55
3daf7dc to
203d331
Compare
Member
|
Thanks for the contribution and the analysis. |
eed3si9n
reviewed
Jul 25, 2026
eed3si9n
reviewed
Jul 25, 2026
eed3si9n
reviewed
Jul 25, 2026
hoangmaihuy
force-pushed
the
perf/analysis-intern
branch
from
July 26, 2026 05:56
203d331 to
edca486
Compare
sbt keeps one Analysis resident per subproject for the whole session, and structurally-equal values are shared neither across those analyses nor, for api tree nodes, within a single one. On a 582-analysis monorepo corpus, holding every analysis co-resident retains 1906 MB after GC; canonicalizing values as they are read brings that to 1237 MB (-35%). - global weak string pool, interned in BinaryDeserializer.string() - 8 weak-valued name -> UsedName pools, one per UseScope combination, probed by the already-canonical name so a pool hit allocates nothing - both pool types are hand-rolled on ConcurrentHashMap + ReferenceQueue in WeakPools.scala, so this adds no dependency - per-read HashMap dedups value-equality xsbti.api nodes (~95% of Type duplication is intra-analysis); it dies with the read, so it cannot leak - share the 8 possible scope EnumSets instead of copying one per UsedName - cache enum values(), which clones its array on every call, on per-node read paths - intern on the fresh-compilation path too (AnalysisCallback.usedName) Canonical values are held weakly, so they are released once no analysis references them, and draining the reference queue releases the pool entry with them. NameHash is deliberately not interned: it has zero within-analysis duplication and a 6.7M distinct population, so a weak pool costs more than it saves (+12 MB retained and 15.6% of read CPU when measured directly). The analysis format version is unchanged and api hashes are preserved, so existing analysis files stay readable and incremental invalidation is unaffected. Read cost is +3.7% on AnalysisFormatBenchmark and +17% loading the corpus, unchanged from 1 to 8 threads: the pools add per-call work, not contention. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
hoangmaihuy
force-pushed
the
perf/analysis-intern
branch
from
July 26, 2026 05:59
edca486 to
be5354d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Disclaimer: Developed with Claude Code and human review in the loop
Fixes #1753
Changes
BinaryDeserializer.string(), so the per-read string table and all of its back-references hold the cross-analysis-shared instance.UsedNamepools — 8 weak-valuedname → UsedNamepools, one perUseScopecombination. AUsedNameis fully determined by its (already canonical) name and 3 scope bits, so probing is keyed by the name string and a pool hit allocates nothing; a candidate is constructed only on a miss.UsedNamealso gains a cachedhashCode(it is re-hashed into a per-classSeton every read) which fits in existing object padding.zinc-core/WeakPools.scalaonConcurrentHashMap+ReferenceQueue, so this adds no dependency.WeakInternerkeys its map by a weak reference that hashes and compares by its referent's value;WeakValuePoolkeeps strong equals-keyed keys with weakly-held values, which is what makes probe-before-construct possible.HashMaponDeserializer, used throughConsistentAnalysisFormat.internNode, dedups value-equalityxsbti.apinodes within a single read. ~95% ofType/TypeParameter/Annotationduplication is intra-analysis, so this captures nearly all of it with no weak-reference bookkeeping, and it is leak-free by construction: the cache dies with the read.StructureandEmptyTypeare excluded (identity equality / singleton).EnumSets — the 8 possibleUseScopesets are allocated once and shared instead of copied perUsedName. Worth 251 MB by itself, independent of interning.enum.values()—values()clones its backing array on every call and sits on per-node read paths (UseScope,Severity,CompileOrder,DefinitionType,Variance,ParameterModifier).readUsedNameSetinterns per name instead of materializing aSeq[String]per scope group and mapping it into freshUsedNames;writeUsedNameSetshares the same scope-bit encoding instead of repeating it.AnalysisCallback.usedName), so compiler-produced names are canonical, not only deserialized ones.Leak-freedom
A build server runs for days, so pooling must not retain. Both pools hold their canonical values weakly: once the last analysis referencing a value is dropped the value becomes collectable, and every pool operation drains the reference queue so the entry goes with it.
WeakInterner(strings) keys the map by a weak reference that hashes and compares by the value of its referent, so nothing in the pool strongly references an interned string.WeakValuePool(UsedName) is keyed strongly by the name and holds the value weakly; the entry remembers its key so that expunging a dead value releases the key too. (Keying that pool weakly instead would leak — the value references its key, pinning it.)WeakPoolSpecasserts, for both pools, that values and keys are released once unreachable;AnalysisInternerSpecasserts the same end to end, plus retention while a reference is held.Thread safety: the pools are
ConcurrentHashMaps, and the fresh-compile path interns from many threads at once; a 16-thread test asserts every thread converges on one canonical instance without deadlock.Transparency: a test asserts api hashes are identical before and after a round trip, so nothing incremental compilation observes changes.
Why
NameHashis deliberately not internedIt is the highest-volume type (15.7 M instances, 424 MB incl. arrays) and the obvious candidate, but the shape is wrong: its internal side has zero within-analysis duplication (5,999,323 occurrences = 5,999,323 distinct values) and the corpus-wide distinct population is 6.7 M, so a weak pool would need ~6.7 M entries at ~40 B each — more than the duplicates it removes. Measured directly, interning it increased retained heap (+12 MB at 100 analyses) and cost 15.6% of read CPU. Unlike
UsedNamethere is also no probe-before-construct trick, because identity includes the api hash int, so all 15.7 M reads would have to allocate a candidate first.It remains the largest analysis-side consumer and is the next thing worth attacking, but it needs a representation change (parallel
String[]/int[]instead of 15.7 M wrappers, or sharing wholenameHashesarrays per(className, apiHash)) — both breaking changes to a publicxsbtitype, so out of scope here.Benchmark results
All numbers below are from one session on one machine, each measurement paired against a stock
developworktree running the identical harness back to back.Heap: all 582 analyses co-resident
4 GB fixed heap, loading every analysis in the corpus and holding strong references to all of them. Both heap figures come from the same run: first sampled with no explicit GC (what the process actually occupies right after loading, floating garbage included), then after four forced GCs with every analysis still held (the live set — the steady-state floor a long-lived build server sits at).
developJudge this by the retained row. It is stable: stock measured 1906 MB in two sessions a day apart, and a second harness reports 1904 MB (1 thread) / 1901 MB (8 threads) for stock against 1244 / 1238 MB for this PR — agreement to within 0.6%. The pre-GC row is that same live set plus whatever floating garbage the loader happened to leave behind, which depends entirely on when G1 last collected: across repeat runs stock landed between 2233 and 2536 MB and this PR between 1460 and 1738 MB, i.e. a delta anywhere from −26% to −39%. Of the 669 MB retained saving, roughly 256 MB comes from the format-level fixes alone (measured with interning disabled while it was still switchable) and the rest from the pools.
Where the heap goes, class by class (pre-GC histogram, same pair of runs):
java.util.RegularEnumSetsbt.internal.inc.UsedNamebyte[](string bodies)xsbti.api.Projectionxsbti.api.SingletonreadUsedNameSetwrappers (JSetWrapper,SetHasAsScala, lambda)EnumSet.asScalaper name[Lxsbti.UseScope;values()java.lang.Stringxsbti.api.Annotation+ args + arraysxsbti.api.NameHash[Lxsbti.api.NameHash;WeakValue,KeyedWeakValue, their map nodes)NameHashand its arrays are byte-identical between the two columns (15,727,923 instances, 377,470,152 bytes), which is the clearest confirmation that this PR leaves it untouched.The interned
UsedNamecount equals the corpus-global distinct population exactly (396,197) — the pools are perfect. Bookkeeping is what interning costs: one weak reference plus one map node per canonical value — 1,007,113 strings (30.7 MiB ofWeakValue) and 396,197UsedNames (12.1 MiB ofKeyedWeakValue), plus ~42.8 MiB ofConcurrentHashMapnodes.CPU: zinc's own
AnalysisFormatBenchmarkRead path, the only one that interns, with enough iterations to settle (
-f1 -wi 5 -i 10 …readConsistentBinary):readConsistentBinarydevelopThe whole benchmark class as a control (
-f1 -wi 3 -i 5 xsbt.AnalysisFormatBenchmark, same session):developreadConsistentBinarywriteConsistentBinarywriteConsistentBinaryNoSortwriteNullwriteNullNoSortThis PR barely touches serialization, so the four write benchmarks are a control — and they come out faster here, which is the point: at five iterations the run-to-run drift on this machine is several percent in either direction, so read cost should be read off the tighter run above (+3.7%), not from this table.
CPU: loading the whole corpus
582 analyses in a 4 GB heap, median of three runs each:
developBoth builds scale identically from 1 to 8 threads — stock 2.02×, this PR 2.04× — so the pools add per-call work, not lock contention:
ReferenceQueue.poll()returns on a plain volatile read while the queue is empty, and the maps areConcurrentHashMaps. (The Guava-based revision measured 2387 ms at 8 threads, indistinguishable from the hand-rolled 2383 ms.)End to end in a real build
Published this branch as zinc
2.0.0-intern-SNAPSHOT, built sbt against it, and ranTest / compileIncrementalover a 303-module monorepo (603 analyses). This run predates the switch from Guava to the hand-rolled pools; the pooled populations it reports are a property of the pooling scheme, which is unchanged.[success], 710 s, no errors.-Xmx20G -Xms8G, G1).UsedNameheld 400,158 instances / 9.2 MiB — within 1% of the corpus-global distinct population — while compiling new modules and holding 600+ analyses. So the pools stay canonical under a real parallel compile, not only when reading files.RegularEnumSetwas absent from the entire histogram.NameHashwas 704 MiB / 30.8 M instances (+125 MiB of arrays), again the dominant analysis-side cost and untouched here.Compatibility
AnalysisInternerissbt.internal.inc.ConcurrentHashMapandReferenceQueue.Testing
WeakPoolSpec— canonicalization and separation of unequal values; 16-thread convergence; release of values once unreachable; for the weak-valued pool,get/putIfAbsentsemantics and release of the key along with its dead value.AnalysisInternerSpec— string canonicalization; oneUsedNameper (name, scope) with probe-before-construct; scope-combination distinction; control-character names pooled under their escaped form; 16-thread convergence; release once unreachable; retention while referenced.ConsistentAnalysisFormatInternerSuite— strings shared across independent reads; node cache within one read;UsedNames shared across independent reads; api hashes preserved.zincRoot/testFull— 221 scalatest tests + 78 property checks, 0 failures, including the e2eIncrementalCompilerSpec/MultiProjectIncrementalSpecsuites that exercise fresh compilation.scalafmtCheckAll,scalafmtSbtCheck,headerCheckandTest/headerCheckclean.