From 4118ee2b56fd580f1779b76b33ec32d06c3cd475 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mai=20Huy=20Ho=C3=A0ng?= Date: Sat, 25 Jul 2026 13:25:15 +0700 Subject: [PATCH] [2.0.x] 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 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) --- .../sbt/internal/inc/AnalysisInterner.scala | 91 ++++++++++++++ .../scala/sbt/internal/inc/Incremental.scala | 11 +- .../scala/sbt/internal/inc/UsedName.scala | 13 +- .../scala/sbt/internal/inc/WeakPools.scala | 117 ++++++++++++++++++ .../internal/inc/AnalysisInternerSpec.scala | 101 +++++++++++++++ .../scala/sbt/internal/inc/WeakPoolSpec.scala | 98 +++++++++++++++ .../consistent/ConsistentAnalysisFormat.scala | 99 +++++++++------ .../internal/inc/consistent/Serializer.scala | 12 +- ...onsistentAnalysisFormatInternerSuite.scala | 100 +++++++++++++++ 9 files changed, 598 insertions(+), 44 deletions(-) create mode 100644 internal/zinc-core/src/main/scala/sbt/internal/inc/AnalysisInterner.scala create mode 100644 internal/zinc-core/src/main/scala/sbt/internal/inc/WeakPools.scala create mode 100644 internal/zinc-core/src/test/scala/sbt/internal/inc/AnalysisInternerSpec.scala create mode 100644 internal/zinc-core/src/test/scala/sbt/internal/inc/WeakPoolSpec.scala create mode 100644 internal/zinc-persist/src/test/scala/sbt/inc/consistent/ConsistentAnalysisFormatInternerSuite.scala diff --git a/internal/zinc-core/src/main/scala/sbt/internal/inc/AnalysisInterner.scala b/internal/zinc-core/src/main/scala/sbt/internal/inc/AnalysisInterner.scala new file mode 100644 index 0000000000..b1fad6eaeb --- /dev/null +++ b/internal/zinc-core/src/main/scala/sbt/internal/inc/AnalysisInterner.scala @@ -0,0 +1,91 @@ +/* + * Zinc - The incremental compiler for Scala. + * Copyright Scala Center, Lightbend, and Mark Harrah + * + * Licensed under Apache License 2.0 + * SPDX-License-Identifier: Apache-2.0 + * + * See the NOTICE file distributed with this work for + * additional information regarding copyright ownership. + */ + +package sbt.internal.inc + +import java.util.{ EnumSet => JEnumSet } + +import xsbti.UseScope + +/** + * The process-wide canonicalizer for strings and `UsedName`s: fresh + * (compiler-produced) and persisted (deserialized) analyses both route through + * it, so co-resident analyses share a single instance per distinct value. + * + * Strings are canonicalized through a weak interner: canonical instances are + * held only through weak references, so once every analysis referencing a + * string is released the canonical instance becomes eligible for GC. + * + * `UsedName`s are canonicalized without constructing a candidate first: a + * `UsedName` is fully determined by its (already canonical) name string and one + * of the eight possible `UseScope` combinations, so each combination gets a + * weak-valued name-keyed pool. A pool entry is removed once the canonical + * `UsedName` is no longer referenced by any analysis. On a pool hit no + * `UsedName` is constructed. + * + * Both pools make interning leak-free across the lifetime of a long-running + * build server. Benchmarks that need an uninterned baseline compare against a + * stock zinc checkout; production code always interns. + */ +object AnalysisInterner { + + private[inc] final val DEFAULT_SCOPE = 1 + private[inc] final val IMPLICIT_SCOPE = 2 + private[inc] final val PAT_MAT_TARGET_SCOPE = 4 + private final val SCOPE_COMBINATIONS = 8 // every subset of the three scopes above + + private val stringPool = new WeakInterner[String] + + // One weak-valued pool per UseScope combination, keyed by the canonical name. + private val usedNamePools: Array[WeakValuePool[String, UsedName]] = + Array.fill(SCOPE_COMBINATIONS)(new WeakValuePool[String, UsedName]) + + def internString(s: String): String = stringPool.intern(s) + + /** + * The canonical `UsedName` for `name` used in the scope combination + * `scopeBits`, an or-ing of `DEFAULT_SCOPE`, `IMPLICIT_SCOPE` and + * `PAT_MAT_TARGET_SCOPE`. Constructs a `UsedName` only when the value is not + * pooled yet. + */ + def usedName(name: String, scopeBits: Int): UsedName = { + // UsedName stores the escaped name, so the pool must be probed with it too; + // escaping is a no-op unless the name contains control characters. + val escaped = UsedName.escapeControlChars(name) + val pool = usedNamePools(scopeBits) + val existing = pool.get(escaped) + if (existing != null) existing + else { + val fresh = UsedName.make(escaped, scopeSets(scopeBits)) + val prev = pool.putIfAbsent(fresh.name, fresh) + if (prev == null) fresh else prev + } + } + + /** + * The eight possible scope sets, indexed by scope bits. Shared by every + * UsedName and treated as immutable -- they must never be mutated. + */ + private[inc] val scopeSets: Array[JEnumSet[UseScope]] = + Array.tabulate(SCOPE_COMBINATIONS) { bits => + val scopes = JEnumSet.noneOf(classOf[UseScope]) + if ((bits & DEFAULT_SCOPE) != 0) scopes.add(UseScope.Default) + if ((bits & IMPLICIT_SCOPE) != 0) scopes.add(UseScope.Implicit) + if ((bits & PAT_MAT_TARGET_SCOPE) != 0) scopes.add(UseScope.PatMatTarget) + scopes + } + + /** The inverse of `scopeSets`: also the encoding used by the persisted format. */ + private[inc] def scopeBits(scopes: JEnumSet[UseScope]): Int = + (if (scopes.contains(UseScope.Default)) DEFAULT_SCOPE else 0) | + (if (scopes.contains(UseScope.Implicit)) IMPLICIT_SCOPE else 0) | + (if (scopes.contains(UseScope.PatMatTarget)) PAT_MAT_TARGET_SCOPE else 0) +} diff --git a/internal/zinc-core/src/main/scala/sbt/internal/inc/Incremental.scala b/internal/zinc-core/src/main/scala/sbt/internal/inc/Incremental.scala index cee416f74a..b29a93c488 100644 --- a/internal/zinc-core/src/main/scala/sbt/internal/inc/Incremental.scala +++ b/internal/zinc-core/src/main/scala/sbt/internal/inc/Incremental.scala @@ -921,9 +921,18 @@ private final class AnalysisCallback( } def usedName(className: String, name: String, useScopes: EnumSet[UseScope]) = { + // Canonicalize freshly-produced used names and their strings into the + // process-wide pools, so a just-compiled analysis shares them with every + // other resident analysis. (Api tree nodes are deduped only when an + // analysis is deserialized, not on this fresh-compile path.) usedNames .getOrElseUpdate(className, ConcurrentHashMap.newKeySet[UsedName].asScala) - .add(UsedName.make(name, useScopes)) + .add( + AnalysisInterner.usedName( + AnalysisInterner.internString(name), + AnalysisInterner.scopeBits(useScopes) + ) + ) () } diff --git a/internal/zinc-core/src/main/scala/sbt/internal/inc/UsedName.scala b/internal/zinc-core/src/main/scala/sbt/internal/inc/UsedName.scala index 4c42137c36..459e6e6a52 100644 --- a/internal/zinc-core/src/main/scala/sbt/internal/inc/UsedName.scala +++ b/internal/zinc-core/src/main/scala/sbt/internal/inc/UsedName.scala @@ -13,12 +13,23 @@ package sbt.internal.inc import java.{ util => ju } import scala.{ collection => sc } +import scala.util.hashing.MurmurHash3 import xsbti.compile.{ UsedName => XUsedName } import xsbti.UseScope +/** + * `scopes` must never be mutated after construction: instances may share one + * scope set (`make` uses the set it is given, and interning aliases equal + * instances), and the hash below is computed once. + */ case class UsedName private (name: String, scopes: ju.EnumSet[UseScope]) extends XUsedName { override def getName: String = name override def getScopes: ju.EnumSet[UseScope] = scopes + + // A canonical (interned) instance is inserted into one name-set per class that + // uses it, and EnumSet.hashCode iterates its elements; caching makes each + // re-hash a field read. Fits in the object's existing alignment padding. + override val hashCode: Int = MurmurHash3.caseClassHash(this) } object UsedName { @@ -33,7 +44,7 @@ object UsedName { new UsedName(escapedName, useScopes) } - private def escapeControlChars(name: String) = { + private[inc] def escapeControlChars(name: String): String = { if (name.indexOf('\n') > 0) // optimize for common case to regex overhead name.replace("\n", "\u26680A") else diff --git a/internal/zinc-core/src/main/scala/sbt/internal/inc/WeakPools.scala b/internal/zinc-core/src/main/scala/sbt/internal/inc/WeakPools.scala new file mode 100644 index 0000000000..ab13a33722 --- /dev/null +++ b/internal/zinc-core/src/main/scala/sbt/internal/inc/WeakPools.scala @@ -0,0 +1,117 @@ +/* + * Zinc - The incremental compiler for Scala. + * Copyright Scala Center, Lightbend, and Mark Harrah + * + * Licensed under Apache License 2.0 + * SPDX-License-Identifier: Apache-2.0 + * + * See the NOTICE file distributed with this work for + * additional information regarding copyright ownership. + */ + +package sbt.internal.inc + +import java.lang.ref.{ ReferenceQueue, WeakReference } +import java.util.concurrent.ConcurrentHashMap + +import scala.annotation.tailrec + +/** + * Canonicalizing pool: `intern` returns one instance per distinct value + * (`equals`/`hashCode`), and holds it only through a weak reference, so a value + * becomes collectable as soon as the last caller drops it. Dead entries are + * expunged by the next pool operation. + */ +private[inc] final class WeakInterner[A <: AnyRef] { + private val stale = new ReferenceQueue[A] + private val pool = new ConcurrentHashMap[WeakValue[A], WeakValue[A]] + + def intern(a: A): A = { + expunge() + val candidate = new WeakValue(a, stale) + @tailrec def publish(): A = pool.putIfAbsent(candidate, candidate) match { + case null => a + case existing => + existing.get match { + case null => // collected since it matched: drop the dead entry and retry + pool.remove(existing, existing) + publish() + case canonical => + candidate.clear() // never enqueue a reference that was not pooled + canonical + } + } + publish() + } + + @tailrec private def expunge(): Unit = stale.poll() match { + case null => () + case dead => + pool.remove(dead, dead) + expunge() + } +} + +/** + * Map that holds its values only through weak references: an entry disappears + * once its value is unreachable, releasing the strong reference to its key with + * it. Unlike an interner it is keyed by something cheaper than the value, so + * callers can probe the pool before constructing a candidate. + */ +private[inc] final class WeakValuePool[K, V <: AnyRef] { + private val stale = new ReferenceQueue[V] + private val pool = new ConcurrentHashMap[K, KeyedWeakValue[K, V]] + + /** The pooled value for `key`, or `null` if there is none. */ + def get(key: K): V = { + expunge() + pool.get(key) match { + case null => null.asInstanceOf[V] + case entry => entry.get + } + } + + /** Pools `value` under `key`, returning `null`, or the value already pooled. */ + @tailrec def putIfAbsent(key: K, value: V): V = { + val candidate = new KeyedWeakValue(key, value, stale) + pool.putIfAbsent(key, candidate) match { + case null => null.asInstanceOf[V] + case existing => + existing.get match { + case null => // collected since it was published: replace the dead entry + pool.remove(key, existing) + putIfAbsent(key, value) + case pooled => + candidate.clear() // never enqueue a reference that was not pooled + pooled + } + } + } + + @tailrec private def expunge(): Unit = stale.poll() match { + case null => () + case dead => + pool.remove(dead.asInstanceOf[KeyedWeakValue[K, V]].key, dead) + expunge() + } +} + +/** Weak reference that hashes and compares by the value of its referent. */ +private final class WeakValue[A <: AnyRef](a: A, stale: ReferenceQueue[A]) + extends WeakReference[A](a, stale) { + private val hash: Int = a.hashCode + + override def hashCode(): Int = hash + override def equals(other: Any): Boolean = other match { + case that: WeakValue[?] => + (this `eq` that) || { + val value = get + value != null && value == that.get + } + case _ => false + } +} + +/** Weak reference that remembers the key its value was pooled under. */ +private final class KeyedWeakValue[K, V <: AnyRef](val key: K, v: V, stale: ReferenceQueue[V]) + extends WeakReference[V](v, stale) diff --git a/internal/zinc-core/src/test/scala/sbt/internal/inc/AnalysisInternerSpec.scala b/internal/zinc-core/src/test/scala/sbt/internal/inc/AnalysisInternerSpec.scala new file mode 100644 index 0000000000..b8a9e35e20 --- /dev/null +++ b/internal/zinc-core/src/test/scala/sbt/internal/inc/AnalysisInternerSpec.scala @@ -0,0 +1,101 @@ +/* + * Zinc - The incremental compiler for Scala. + * Copyright Scala Center, Lightbend, and Mark Harrah + * + * Licensed under Apache License 2.0 + * SPDX-License-Identifier: Apache-2.0 + * + * See the NOTICE file distributed with this work for + * additional information regarding copyright ownership. + */ + +package sbt.internal.inc + +import java.lang.ref.WeakReference +import java.util.concurrent.{ Callable, CountDownLatch, Executors, TimeUnit } +import xsbti.UseScope + +class AnalysisInternerSpec extends UnitSpec { + + behavior of "AnalysisInterner" + + it should "canonicalize equal strings to one instance" in { + val s1 = new String("com.example.Foo") // force distinct heap instances + val s2 = new String("com.example.Foo") + assert(s1 `ne` s2) + assert(AnalysisInterner.internString(s1) `eq` AnalysisInterner.internString(s2)) + } + + it should "return one UsedName instance per (name, scope) without construction on hit" in { + val a = AnalysisInterner.usedName("map", 1) + val b = AnalysisInterner.usedName(new String("map"), 1) + assert(a `eq` b) + assert(a.name == "map") + assert(a.scopes.contains(UseScope.Default)) + assert(!a.scopes.contains(UseScope.Implicit)) + } + + it should "distinguish scope combinations for the same name" in { + val default = AnalysisInterner.usedName("map", 1) + val implicitScope = AnalysisInterner.usedName("map", 2) + val patMat = AnalysisInterner.usedName("map", 4) + assert(default `ne` implicitScope) + assert(default `ne` patMat) + assert(implicitScope.scopes.contains(UseScope.Implicit)) + assert(patMat.scopes.contains(UseScope.PatMatTarget)) + } + + it should "pool names containing control characters under their escaped form" in { + val a = AnalysisInterner.usedName("weird\nname", 1) + val b = AnalysisInterner.usedName("weird\nname", 1) + assert(a `eq` b) + assert(a.name == "weird♨0Aname") // stored escaped, exactly as UsedName.make does + } + + it should "converge on one canonical instance under concurrent interning" in { + // Parallel compilation interns from many threads at once. Every thread must + // observe the same canonical reference, and the pools must never deadlock. + val threads = 16 + val start = new CountDownLatch(1) + val pool = Executors.newFixedThreadPool(threads) + try { + val futures = (1 to threads).map { _ => + pool.submit(new Callable[(String, UsedName)] { + def call(): (String, UsedName) = { + start.await() // release all threads together to maximize contention + ( + AnalysisInterner.internString(new String("concurrent.value")), + AnalysisInterner.usedName(new String("concurrent.value"), 5) + ) + } + }) + } + start.countDown() + val results = futures.map(_.get(30, TimeUnit.SECONDS)).toList // deadlock => timeout + assert(results.forall(_._1 `eq` results.head._1)) + assert(results.forall(_._2 `eq` results.head._2)) + } finally pool.shutdownNow() + } + + it should "release canonical instances once no analysis references them" in { + // The leak-free requirement: canonical instances are held only weakly, so + // once every analysis referencing a value is dropped it becomes GC-eligible. + val stringRef = + new WeakReference[String](AnalysisInterner.internString(new String("ephemeral.str"))) + val usedNameRef = + new WeakReference[UsedName](AnalysisInterner.usedName("ephemeral.used", 1)) + val released = (1 to 100).exists { _ => + System.gc() + Thread.sleep(10) + stringRef.get() == null && usedNameRef.get() == null + } + assert(released, "weak pools must not retain values after they become unreachable") + } + + it should "keep pooled values while an analysis still references them" in { + val held = AnalysisInterner.usedName("held", 1) + System.gc() + Thread.sleep(10) + assert(AnalysisInterner.usedName("held", 1) `eq` held) + } +} diff --git a/internal/zinc-core/src/test/scala/sbt/internal/inc/WeakPoolSpec.scala b/internal/zinc-core/src/test/scala/sbt/internal/inc/WeakPoolSpec.scala new file mode 100644 index 0000000000..fa3f5df1ef --- /dev/null +++ b/internal/zinc-core/src/test/scala/sbt/internal/inc/WeakPoolSpec.scala @@ -0,0 +1,98 @@ +/* + * Zinc - The incremental compiler for Scala. + * Copyright Scala Center, Lightbend, and Mark Harrah + * + * Licensed under Apache License 2.0 + * SPDX-License-Identifier: Apache-2.0 + * + * See the NOTICE file distributed with this work for + * additional information regarding copyright ownership. + */ + +package sbt.internal.inc + +import java.lang.ref.WeakReference +import java.util.concurrent.{ Callable, CountDownLatch, Executors, TimeUnit } + +class WeakPoolSpec extends UnitSpec { + + /** GC is not obliged to collect on the first attempt, so retry for a while. */ + private def gcUntil(cond: => Boolean): Boolean = + (1 to 100).exists { _ => + System.gc() + Thread.sleep(10) + cond + } + + behavior of "WeakInterner" + + it should "return one canonical instance per equal value" in { + val interner = new WeakInterner[String] + val first = interner.intern(new String("value")) // force distinct heap instances + val second = interner.intern(new String("value")) + assert(first `eq` second) + assert(first == "value") + } + + it should "keep values that are not equal apart" in { + val interner = new WeakInterner[String] + assert(interner.intern("a") `ne` interner.intern("b")) + } + + it should "release canonical instances once they are unreachable" in { + val interner = new WeakInterner[String] + val ref = new WeakReference(interner.intern(new String("ephemeral"))) + // Any pool operation drains the reference queue; without that the pool would + // keep growing for the lifetime of a build server. + assert(gcUntil { interner.intern("probe"); ref.get() == null }) + } + + it should "converge on one instance under concurrent interning" in { + val interner = new WeakInterner[String] + val threads = 16 + val start = new CountDownLatch(1) + val executor = Executors.newFixedThreadPool(threads) + try { + val futures = (1 to threads).map { _ => + executor.submit(new Callable[String] { + def call(): String = { + start.await() // release all threads together to maximize contention + interner.intern(new String("contended")) + } + }) + } + start.countDown() + val results = futures.map(_.get(30, TimeUnit.SECONDS)).toList // deadlock => timeout + assert(results.forall(_ `eq` results.head)) + } finally executor.shutdownNow() + } + + behavior of "WeakValuePool" + + it should "return null for an absent key" in { + val pool = new WeakValuePool[String, StringBuilder] + assert(pool.get("absent") == null) + } + + it should "keep the first value published for a key" in { + val pool = new WeakValuePool[String, StringBuilder] + val first = new StringBuilder("v") + assert(pool.putIfAbsent("k", first) == null) + assert(pool.putIfAbsent("k", new StringBuilder("v")) `eq` first) + assert(pool.get("k") `eq` first) + } + + it should "drop the entry, including its key, once the value is unreachable" in { + val pool = new WeakValuePool[String, StringBuilder] + var key: String = new String("transient.key") // a literal would live in the string table + var value: StringBuilder = new StringBuilder("transient.value") + pool.putIfAbsent(key, value) + val keyRef = new WeakReference(key) + val valueRef = new WeakReference(value) + key = null + value = null + // The entry holds its key strongly, so releasing the value must release the + // key with it -- otherwise the pool leaks one key per dead value. + assert(gcUntil { pool.get("other"); keyRef.get() == null && valueRef.get() == null }) + } +} diff --git a/internal/zinc-persist/src/main/scala/sbt/internal/inc/consistent/ConsistentAnalysisFormat.scala b/internal/zinc-persist/src/main/scala/sbt/internal/inc/consistent/ConsistentAnalysisFormat.scala index 5fc2b283fc..75dd406ecc 100644 --- a/internal/zinc-persist/src/main/scala/sbt/internal/inc/consistent/ConsistentAnalysisFormat.scala +++ b/internal/zinc-persist/src/main/scala/sbt/internal/inc/consistent/ConsistentAnalysisFormat.scala @@ -12,7 +12,7 @@ package sbt.internal.inc.consistent import java.nio.file.Paths -import java.util.{ Arrays, Comparator, EnumSet } +import java.util.{ Arrays, Comparator } import sbt.internal.inc.{ UsedName, Stamp => StampImpl, _ } import sbt.internal.util.Relation import sbt.util.InterfaceUtil @@ -192,7 +192,7 @@ class ConsistentAnalysisFormat(val mappers: ReadWriteMappers, reproducible: Bool val bh = in.long() val ebh = in.long() val nhNames = in.readStringArray() - val nhScopes = in.readArray[UseScope]() { UseScope.values()(in.byte().toInt) } + val nhScopes = in.readArray[UseScope]() { useScopeValues(in.byte().toInt) } val nhHashes = in.readArray[Int]() { in.int() } val nameHashes = new Array[NameHash](nhNames.length) var i = 0 @@ -265,7 +265,7 @@ class ConsistentAnalysisFormat(val mappers: ReadWriteMappers, reproducible: Bool private def readSourceInfos(in: Deserializer): SourceInfos = { def readProblem(): Problem = in.readBlock { val category = in.string() - val severity = Severity.values.apply(in.byte().toInt) + val severity = severityValues(in.byte().toInt) val message = in.string() val rendered = Option(in.string()) def io(): Option[Integer] = in.int() match { case -1 => None; case i => Some(i) } @@ -339,7 +339,7 @@ class ConsistentAnalysisFormat(val mappers: ReadWriteMappers, reproducible: Bool val scalacOptions = in.readArray() { readMapper.mapScalacOption(in.string()) } val javacOptions = in.readArray() { readMapper.mapJavacOption(in.string()) } val compilerVersion = in.string() - val compileOrder = CompileOrder.values()(in.byte().toInt) + val compileOrder = compileOrderValues(in.byte().toInt) val skipApiStoring = in.bool() val extra = in.readArray(2) { InterfaceUtil.t2(in.string() -> in.string()) } val outputPath = in.string() @@ -434,12 +434,7 @@ class ConsistentAnalysisFormat(val mappers: ReadWriteMappers, reproducible: Bool private def writeUsedNameSet(out: Serializer, uns: scala.collection.Set[UsedName]): Unit = { out.writeBlock("UsedName") { val groups0 = uns.iterator.map { un => - val sc = un.scopes - var i = 0 - if (sc.contains(UseScope.Default)) i += 1 - if (sc.contains(UseScope.Implicit)) i += 2 - if (sc.contains(UseScope.PatMatTarget)) i += 4 - (un.name, i.toByte) + (un.name, AnalysisInterner.scopeBits(un.scopes).toByte) }.toArray.groupBy(_._2) val groups = if (reproducible) groups0.toVector.sortBy(_._1) else groups0 out.writeColl("groups", groups, 2) { case (g, gNames) => @@ -452,12 +447,14 @@ class ConsistentAnalysisFormat(val mappers: ReadWriteMappers, reproducible: Bool } private def readUsedNameSet(in: Deserializer): Set[UsedName] = { - import scala.jdk.CollectionConverters.* in.readBlock { + // The name and the scope bits fully determine a UsedName, so the interner + // can return the pooled instance without constructing a candidate first. val data = in.readColl[Vector[UsedName], Vector[Vector[UsedName]]](Vector, 2) { - val i = in.byte().toInt - val names = in.readStringSeq() - names.iterator.map { n => UsedName(n, useScopes(i).asScala) }.toVector + val scopeBits = in.byte().toInt + in.readColl[UsedName, Vector[UsedName]](Vector) { + AnalysisInterner.usedName(in.string(), scopeBits) + } } data.flatten.toSet } @@ -542,14 +539,14 @@ class ConsistentAnalysisFormat(val mappers: ReadWriteMappers, reproducible: Bool private def readAnnotation(in: Deserializer): Annotation = in.readBlock { val base = readType(in) val args = in.readArray(2)(AnnotationArgument.of(in.string(), in.string())) - Annotation.of(base, args) + internNode(in, Annotation.of(base, args)) } private def writeDefinitionType(out: Serializer, dt: DefinitionType): Unit = out.byte(dt.ordinal().toByte) private def readDefinitionType(in: Deserializer): DefinitionType = - DefinitionType.values()(in.byte().toInt) + definitionTypeValues(in.byte().toInt) private def writeTypeParameter(out: Serializer, tp: TypeParameter): Unit = out.writeBlock("TypeParameter") { @@ -562,13 +559,16 @@ class ConsistentAnalysisFormat(val mappers: ReadWriteMappers, reproducible: Bool } private def readTypeParameter(in: Deserializer): TypeParameter = in.readBlock { - TypeParameter.of( - in.string(), - in.readArray[Annotation]()(readAnnotation(in)), - in.readArray[TypeParameter]()(readTypeParameter(in)), - Variance.values()(in.byte().toInt), - readType(in), - readType(in) + internNode( + in, + TypeParameter.of( + in.string(), + in.readArray[Annotation]()(readAnnotation(in)), + in.readArray[TypeParameter]()(readTypeParameter(in)), + varianceValues(in.byte().toInt), + readType(in), + readType(in) + ) ) } @@ -612,16 +612,21 @@ class ConsistentAnalysisFormat(val mappers: ReadWriteMappers, reproducible: Bool } private def readType(in: Deserializer): Type = in.readBlock { + // Structure (case 2) uses identity equality (lazy members), so dedup + // cannot canonicalize it; every other variant has value equality. + def i(t: Type): Type = internNode(in, t) in.byte() match { - case 0 => ParameterRef.of(in.string()) - case 1 => Parameterized.of(readType(in), in.readArray[Type]()(readType(in))) + case 0 => i(ParameterRef.of(in.string())) + case 1 => i(Parameterized.of(readType(in), in.readArray[Type]()(readType(in)))) case 2 => readStructure(in) - case 3 => Polymorphic.of(readType(in), in.readArray[TypeParameter]()(readTypeParameter(in))) - case 4 => Constant.of(readType(in), in.string()) - case 5 => Existential.of(readType(in), in.readArray[TypeParameter]()(readTypeParameter(in))) - case 6 => Singleton.of(readPath(in)) - case 7 => Projection.of(readType(in), in.string()) - case 8 => Annotated.of(readType(in), in.readArray[Annotation]()(readAnnotation(in))) + case 3 => + i(Polymorphic.of(readType(in), in.readArray[TypeParameter]()(readTypeParameter(in)))) + case 4 => i(Constant.of(readType(in), in.string())) + case 5 => + i(Existential.of(readType(in), in.readArray[TypeParameter]()(readTypeParameter(in)))) + case 6 => i(Singleton.of(readPath(in))) + case 7 => i(Projection.of(readType(in), in.string())) + case 8 => i(Annotated.of(readType(in), in.readArray[Annotation]()(readAnnotation(in)))) case 9 => EmptyTypeSingleton } } @@ -740,7 +745,7 @@ class ConsistentAnalysisFormat(val mappers: ReadWriteMappers, reproducible: Bool in.string(), readType(in), in.bool(), - ParameterModifier.values()(in.byte().toInt) + parameterModifierValues(in.byte().toInt) ) }, in.bool() @@ -813,21 +818,33 @@ class ConsistentAnalysisFormat(val mappers: ReadWriteMappers, reproducible: Bool } object ConsistentAnalysisFormat { + + /** + * Dedups a value-equality `xsbti.api` tree node against the current read only. + * Nearly all node duplication is within a single analysis, so a plain per-read + * map captures it without weak-reference bookkeeping; the cache dies with the + * deserializer, so it cannot leak. + */ + private[consistent] def internNode[A <: AnyRef](in: Deserializer, a: A): A = { + val prev = in.nodeCache.putIfAbsent(a, a) + if (prev == null) a else prev.asInstanceOf[A] + } + private final val EmptyTypeSingleton = EmptyType.of() private final val ThisSingleton = This.of() private final val ThisQualifierSingleton = ThisQualifier.of() private final val UnqualifiedSingleton = Unqualified.of() private final val PublicSingleton = Public.of() - private final val DefaultCompilationTimestamp: Long = 1262304042000L // 2010-01-01T00:00:42Z - private final val useScopes: Array[EnumSet[UseScope]] = - Array.tabulate(8) { i => - val e = EnumSet.noneOf(classOf[UseScope]) - if ((i & 1) != 0) e.add(UseScope.Default) - if ((i & 2) != 0) e.add(UseScope.Implicit) - if ((i & 4) != 0) e.add(UseScope.PatMatTarget) - e - } + // Enum `values()` clones the backing array on every call; these are on per-node + // read paths, so cache one copy of each. + private final val useScopeValues = UseScope.values() + private final val severityValues = Severity.values() + private final val compileOrderValues = CompileOrder.values() + private final val definitionTypeValues = DefinitionType.values() + private final val varianceValues = Variance.values() + private final val parameterModifierValues = ParameterModifier.values() + private final val DefaultCompilationTimestamp: Long = 1262304042000L // 2010-01-01T00:00:42Z private final val nameHashComparator: Comparator[NameHash] = new Comparator[NameHash] { def compare(o1: NameHash, o2: NameHash): Int = { diff --git a/internal/zinc-persist/src/main/scala/sbt/internal/inc/consistent/Serializer.scala b/internal/zinc-persist/src/main/scala/sbt/internal/inc/consistent/Serializer.scala index 10fa9ba9d4..bc222d0511 100644 --- a/internal/zinc-persist/src/main/scala/sbt/internal/inc/consistent/Serializer.scala +++ b/internal/zinc-persist/src/main/scala/sbt/internal/inc/consistent/Serializer.scala @@ -30,6 +30,8 @@ import scala.collection.mutable import scala.reflect.ClassTag import scala.collection.Factory +import sbt.internal.inc.AnalysisInterner + /** Structural serialization for text and binary formats. */ abstract class Serializer { private final val dedupMap: mutable.Map[AnyRef, Int] = mutable.Map.empty @@ -116,6 +118,11 @@ abstract class Serializer { abstract class Deserializer { private final val dedupBuffer: ArrayBuffer[AnyRef] = ArrayBuffer.empty + // Per-read dedup state for value-equality `xsbti.api` tree nodes (used by + // ConsistentAnalysisFormat.internNode): dies with the deserializer, so it + // cannot leak. + private[consistent] final val nodeCache = new java.util.HashMap[AnyRef, AnyRef]() + def startBlock(): Unit def startArray(): Int def endBlock(): Unit @@ -467,7 +474,7 @@ class BinaryDeserializer(_in: InputStream) extends Deserializer { case -1 => null case 0 => "" case len if len > 0 => - val s = if (len <= buffer.length) { + val raw = if (len <= buffer.length) { ensure(len) val s = new String(buffer, pos, len, StandardCharsets.UTF_8) pos += len @@ -478,6 +485,9 @@ class BinaryDeserializer(_in: InputStream) extends Deserializer { assert(read == len) new String(a, StandardCharsets.UTF_8) } + // Canonicalize the freshly decoded string so the per-read table and all its + // back-references hold the cross-analysis-shared instance. + val s = AnalysisInterner.internString(raw) strings += s s case idx => diff --git a/internal/zinc-persist/src/test/scala/sbt/inc/consistent/ConsistentAnalysisFormatInternerSuite.scala b/internal/zinc-persist/src/test/scala/sbt/inc/consistent/ConsistentAnalysisFormatInternerSuite.scala new file mode 100644 index 0000000000..adfa1cbfbd --- /dev/null +++ b/internal/zinc-persist/src/test/scala/sbt/inc/consistent/ConsistentAnalysisFormatInternerSuite.scala @@ -0,0 +1,100 @@ +/* + * Zinc - The incremental compiler for Scala. + * Copyright Scala Center, Lightbend, and Mark Harrah + * + * Licensed under Apache License 2.0 + * SPDX-License-Identifier: Apache-2.0 + * + * See the NOTICE file distributed with this work for + * additional information regarding copyright ownership. + */ + +package sbt.internal.inc.consistent + +import org.scalatest.funsuite.AnyFunSuite +import sbt.internal.inc.{ Analysis, FileAnalysisStore, UsedName } +import sbt.io.IO +import xsbti.api.{ ParameterRef, Projection } +import xsbti.compile.AnalysisContents +import xsbti.compile.analysis.ReadWriteMappers +import java.io.* + +/** + * Verifies that the consistent binary format dedups inline while deserializing: + * strings and `UsedName`s are shared across independently-read analyses via the + * process-wide interner, and value-equality api tree nodes are shared within one + * read via the per-read node cache. + */ +class ConsistentAnalysisFormatInternerSuite extends AnyFunSuite { + + /** Serialize a single string, then read it back through a fresh deserializer. */ + private def roundTripString(s: String): String = { + val out = new ByteArrayOutputStream() + val ser = SerializerFactory.binary.serializerFor(out) + ser.string(s) + ser.end() + val deser = SerializerFactory.binary.deserializerFor(new ByteArrayInputStream(out.toByteArray)) + deser.string() + } + + test("strings are canonicalized across independent reads (cross-analysis)") { + val a = roundTripString(new String("com.example.CrossAnalysis")) + val b = roundTripString(new String("com.example.CrossAnalysis")) + assert(a == b) + assert(a `eq` b) // two separate reads share one canonical instance + } + + test("api tree nodes are deduped within one read (per-read node cache)") { + val deser = + SerializerFactory.binary.deserializerFor(new ByteArrayInputStream(Array.empty[Byte])) + val t1 = ConsistentAnalysisFormat.internNode(deser, Projection.of(ParameterRef.of("P"), "x")) + val t2 = ConsistentAnalysisFormat.internNode(deser, Projection.of(ParameterRef.of("P"), "x")) + assert(t1 `eq` t2) + } + + private val mappers = ReadWriteMappers.getEmptyMappers() + + private def writeConsistentBinary(contents: AnalysisContents): File = { + val out = File.createTempFile("interner-node", ".zip") + out.deleteOnExit() + if (out.exists()) IO.delete(out) + ConsistentFileAnalysisStore.binary(out, mappers).set(contents) + out + } + + private def readAnalysis(file: File): Analysis = + ConsistentFileAnalysisStore.binary(file, mappers).unsafeGet().getAnalysis.asInstanceOf[Analysis] + + /** First used name under the alphabetically-first class -- deterministic across reads. */ + private def firstUsedName(a: Analysis): UsedName = + a.relations.names.toMultiMap.toSeq + .sortBy(_._1) + .iterator + .flatMap(_._2.toSeq.sortBy(_.name)) + .next() + + test("UsedNames are canonicalized across independent reads (cross-analysis)") { + val d = new File("../../../test-data", "library.zip") + assert(d.exists()) + val bin = writeConsistentBinary(FileAnalysisStore.text(d).unsafeGet()) + + val a = readAnalysis(bin) // two independent deserializations of the same analysis + val b = readAnalysis(bin) + val ua = firstUsedName(a) + val ub = firstUsedName(b) + assert(ua == ub) // same value (same analysis read twice) + assert(ua `eq` ub) // shared canonical instance across the two reads + } + + test("interning preserves apiHash (transparent to change detection)") { + val d = new File("../../../test-data", "library.zip") + assert(d.exists()) + val original = FileAnalysisStore.text(d).unsafeGet().getAnalysis.asInstanceOf[Analysis] + val interned = readAnalysis(writeConsistentBinary(FileAnalysisStore.text(d).unsafeGet())) + + val originalHashes = original.apis.internal.view.mapValues(_.apiHash()).toMap + val internedHashes = interned.apis.internal.view.mapValues(_.apiHash()).toMap + assert(originalHashes.nonEmpty) + assert(originalHashes == internedHashes) // interning changes nothing change-detection observes + } +}