Skip to content

Commit 203d331

Browse files
hoangmaihuyclaude
andcommitted
Intern analysis values while deserializing
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 costs 2233 MB; canonicalizing values as they are read brings that to 1460 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 - 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; read used-name sets with a direct loop - intern on the fresh-compilation path too (AnalysisCallback.usedName) Canonical values are held weakly, so they are released once no analysis references 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 +12% on AnalysisFormatBenchmark and +5% loading the corpus with 8 threads. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 57b581e commit 203d331

8 files changed

Lines changed: 398 additions & 41 deletions

File tree

build.sbt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,8 @@ lazy val zincCore = (projectMatrix in internalPath / "zinc-core")
349349
exclude[MissingClassProblem]("xsbti.*"),
350350
),
351351
libraryDependencies ++= List(
352-
"org.scala-lang.modules" %% "scala-parallel-collections" % "1.2.0"
352+
"org.scala-lang.modules" %% "scala-parallel-collections" % "1.2.0",
353+
"com.google.guava" % "guava" % "33.6.0-jre"
353354
),
354355
)
355356
.jvmPlatform(scalaVersions = scala3_only)
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
/*
2+
* Zinc - The incremental compiler for Scala.
3+
* Copyright Scala Center, Lightbend, and Mark Harrah
4+
*
5+
* Licensed under Apache License 2.0
6+
* SPDX-License-Identifier: Apache-2.0
7+
*
8+
* See the NOTICE file distributed with this work for
9+
* additional information regarding copyright ownership.
10+
*/
11+
12+
package sbt.internal.inc
13+
14+
import java.util.{ EnumSet => JEnumSet }
15+
import java.util.concurrent.ConcurrentMap
16+
17+
import com.google.common.collect.{ Interner, Interners, MapMaker }
18+
import xsbti.UseScope
19+
20+
/**
21+
* The process-wide canonicalizer for strings and `UsedName`s: fresh
22+
* (compiler-produced) and persisted (deserialized) analyses both route through
23+
* it, so co-resident analyses share a single instance per distinct value.
24+
*
25+
* Strings are canonicalized through a Guava weak interner: canonical instances
26+
* are held only through weak references, so once every analysis referencing a
27+
* string is released the canonical instance becomes eligible for GC.
28+
*
29+
* `UsedName`s are canonicalized without constructing a candidate first: a
30+
* `UsedName` is fully determined by its (already canonical) name string and one
31+
* of the eight possible `UseScope` combinations, so each combination gets a
32+
* weak-valued name-keyed map. A pool entry is removed once the canonical
33+
* `UsedName` is no longer referenced by any analysis (the value also holds the
34+
* only strong reference to its key while alive). On a pool hit no `UsedName`
35+
* is constructed.
36+
*
37+
* Both pools make interning leak-free across the lifetime of a long-running
38+
* build server. Benchmarks that need an uninterned baseline compare against a
39+
* stock zinc checkout; production code always interns.
40+
*/
41+
object AnalysisInterner {
42+
43+
private val stringPool: Interner[String] = Interners.newWeakInterner()
44+
45+
// One weak-valued pool per UseScope combination, keyed by the canonical name.
46+
private val usedNamePools: Array[ConcurrentMap[String, UsedName]] =
47+
Array.fill(8)(new MapMaker().weakValues().makeMap[String, UsedName]())
48+
49+
def internString(s: String): String = stringPool.intern(s)
50+
51+
/**
52+
* The canonical `UsedName` for `name` used in the scope combination
53+
* `scopeBits` (bit 0: Default, bit 1: Implicit, bit 2: PatMatTarget).
54+
* Constructs a `UsedName` only when the value is not pooled yet.
55+
*/
56+
def usedName(name: String, scopeBits: Int): UsedName = {
57+
// UsedName stores the escaped name, so the pool must be probed with it too;
58+
// escaping is a no-op unless the name contains control characters.
59+
val escaped = UsedName.escapeControlChars(name)
60+
val pool = usedNamePools(scopeBits)
61+
val existing = pool.get(escaped)
62+
if (existing != null) existing
63+
else {
64+
val fresh = UsedName.make(escaped, scopeSets(scopeBits))
65+
val prev = pool.putIfAbsent(fresh.name, fresh)
66+
if (prev == null) fresh else prev
67+
}
68+
}
69+
70+
/**
71+
* The eight possible scope sets, indexed by scope bits. Shared by every
72+
* UsedName and treated as immutable -- they must never be mutated. The bit
73+
* scheme must stay in sync with `ConsistentAnalysisFormat.writeUsedNameSet`
74+
* (zinc-persist), which encodes the same bits on the write side.
75+
*/
76+
private[inc] val scopeSets: Array[JEnumSet[UseScope]] =
77+
Array.tabulate(8) { i =>
78+
val e = JEnumSet.noneOf(classOf[UseScope])
79+
if ((i & 1) != 0) e.add(UseScope.Default)
80+
if ((i & 2) != 0) e.add(UseScope.Implicit)
81+
if ((i & 4) != 0) e.add(UseScope.PatMatTarget)
82+
e
83+
}
84+
85+
private[inc] def scopeBits(scopes: JEnumSet[UseScope]): Int = {
86+
var i = 0
87+
if (scopes.contains(UseScope.Default)) i |= 1
88+
if (scopes.contains(UseScope.Implicit)) i |= 2
89+
if (scopes.contains(UseScope.PatMatTarget)) i |= 4
90+
i
91+
}
92+
}

internal/zinc-core/src/main/scala/sbt/internal/inc/Incremental.scala

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -920,9 +920,18 @@ private final class AnalysisCallback(
920920
}
921921

922922
def usedName(className: String, name: String, useScopes: EnumSet[UseScope]) = {
923+
// Canonicalize freshly-produced used names and their strings into the
924+
// process-wide pools, so a just-compiled analysis shares them with every
925+
// other resident analysis. (Api tree nodes are deduped only when an
926+
// analysis is deserialized, not on this fresh-compile path.)
923927
usedNames
924928
.getOrElseUpdate(className, ConcurrentHashMap.newKeySet[UsedName].asScala)
925-
.add(UsedName.make(name, useScopes))
929+
.add(
930+
AnalysisInterner.usedName(
931+
AnalysisInterner.internString(name),
932+
AnalysisInterner.scopeBits(useScopes)
933+
)
934+
)
926935
()
927936
}
928937

internal/zinc-core/src/main/scala/sbt/internal/inc/UsedName.scala

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,23 @@ package sbt.internal.inc
1313

1414
import java.{ util => ju }
1515
import scala.{ collection => sc }
16+
import scala.util.hashing.MurmurHash3
1617
import xsbti.compile.{ UsedName => XUsedName }
1718
import xsbti.UseScope
1819

20+
/**
21+
* `scopes` must never be mutated after construction: instances may share one
22+
* scope set (`make` uses the set it is given, and interning aliases equal
23+
* instances), and the hash below is computed once.
24+
*/
1925
case class UsedName private (name: String, scopes: ju.EnumSet[UseScope]) extends XUsedName {
2026
override def getName: String = name
2127
override def getScopes: ju.EnumSet[UseScope] = scopes
28+
29+
// A canonical (interned) instance is inserted into one name-set per class that
30+
// uses it, and EnumSet.hashCode iterates its elements; caching makes each
31+
// re-hash a field read. Fits in the object's existing alignment padding.
32+
override val hashCode: Int = MurmurHash3.caseClassHash(this)
2233
}
2334

2435
object UsedName {
@@ -33,7 +44,7 @@ object UsedName {
3344
new UsedName(escapedName, useScopes)
3445
}
3546

36-
private def escapeControlChars(name: String) = {
47+
private[inc] def escapeControlChars(name: String): String = {
3748
if (name.indexOf('\n') > 0) // optimize for common case to regex overhead
3849
name.replace("\n", "\u26680A")
3950
else
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
/*
2+
* Zinc - The incremental compiler for Scala.
3+
* Copyright Scala Center, Lightbend, and Mark Harrah
4+
*
5+
* Licensed under Apache License 2.0
6+
* SPDX-License-Identifier: Apache-2.0
7+
*
8+
* See the NOTICE file distributed with this work for
9+
* additional information regarding copyright ownership.
10+
*/
11+
12+
package sbt.internal.inc
13+
14+
import java.lang.ref.WeakReference
15+
import java.util.concurrent.{ Callable, CountDownLatch, Executors, TimeUnit }
16+
import xsbti.UseScope
17+
18+
class AnalysisInternerSpec extends UnitSpec {
19+
20+
behavior of "AnalysisInterner"
21+
22+
it should "canonicalize equal strings to one instance" in {
23+
val s1 = new String("com.example.Foo") // force distinct heap instances
24+
val s2 = new String("com.example.Foo")
25+
assert(s1 `ne` s2)
26+
assert(AnalysisInterner.internString(s1) `eq` AnalysisInterner.internString(s2))
27+
}
28+
29+
it should "return one UsedName instance per (name, scope) without construction on hit" in {
30+
val a = AnalysisInterner.usedName("map", 1)
31+
val b = AnalysisInterner.usedName(new String("map"), 1)
32+
assert(a `eq` b)
33+
assert(a.name == "map")
34+
assert(a.scopes.contains(UseScope.Default))
35+
assert(!a.scopes.contains(UseScope.Implicit))
36+
}
37+
38+
it should "distinguish scope combinations for the same name" in {
39+
val default = AnalysisInterner.usedName("map", 1)
40+
val implicitScope = AnalysisInterner.usedName("map", 2)
41+
val patMat = AnalysisInterner.usedName("map", 4)
42+
assert(default `ne` implicitScope)
43+
assert(default `ne` patMat)
44+
assert(implicitScope.scopes.contains(UseScope.Implicit))
45+
assert(patMat.scopes.contains(UseScope.PatMatTarget))
46+
}
47+
48+
it should "pool names containing control characters under their escaped form" in {
49+
val a = AnalysisInterner.usedName("weird\nname", 1)
50+
val b = AnalysisInterner.usedName("weird\nname", 1)
51+
assert(a `eq` b)
52+
assert(a.name == "weird♨0Aname") // stored escaped, exactly as UsedName.make does
53+
}
54+
55+
it should "converge on one canonical instance under concurrent interning" in {
56+
// Parallel compilation interns from many threads at once. Every thread must
57+
// observe the same canonical reference, and the pools must never deadlock.
58+
val threads = 16
59+
val start = new CountDownLatch(1)
60+
val pool = Executors.newFixedThreadPool(threads)
61+
try {
62+
val futures = (1 to threads).map { _ =>
63+
pool.submit(new Callable[(String, UsedName)] {
64+
def call(): (String, UsedName) = {
65+
start.await() // release all threads together to maximize contention
66+
(
67+
AnalysisInterner.internString(new String("concurrent.value")),
68+
AnalysisInterner.usedName(new String("concurrent.value"), 5)
69+
)
70+
}
71+
})
72+
}
73+
start.countDown()
74+
val results = futures.map(_.get(30, TimeUnit.SECONDS)).toList // deadlock => timeout
75+
assert(results.forall(_._1 `eq` results.head._1))
76+
assert(results.forall(_._2 `eq` results.head._2))
77+
} finally pool.shutdownNow()
78+
}
79+
80+
it should "release canonical instances once no analysis references them" in {
81+
// The leak-free requirement: canonical instances are held only weakly, so
82+
// once every analysis referencing a value is dropped it becomes GC-eligible.
83+
val stringRef =
84+
new WeakReference[String](AnalysisInterner.internString(new String("ephemeral.str")))
85+
val usedNameRef =
86+
new WeakReference[UsedName](AnalysisInterner.usedName("ephemeral.used", 1))
87+
val released = (1 to 100).exists { _ =>
88+
System.gc()
89+
Thread.sleep(10)
90+
stringRef.get() == null && usedNameRef.get() == null
91+
}
92+
assert(released, "weak pools must not retain values after they become unreachable")
93+
}
94+
95+
it should "keep pooled values while an analysis still references them" in {
96+
val held = AnalysisInterner.usedName("held", 1)
97+
System.gc()
98+
Thread.sleep(10)
99+
assert(AnalysisInterner.usedName("held", 1) `eq` held)
100+
}
101+
}

0 commit comments

Comments
 (0)