diff --git a/lm-core/src/main/scala/sbt/internal/librarymanagement/JsonUtil.scala b/lm-core/src/main/scala/sbt/internal/librarymanagement/JsonUtil.scala index 59f496b31..7c632dcbd 100644 --- a/lm-core/src/main/scala/sbt/internal/librarymanagement/JsonUtil.scala +++ b/lm-core/src/main/scala/sbt/internal/librarymanagement/JsonUtil.scala @@ -40,26 +40,30 @@ private[sbt] object JsonUtil { oar.organization, oar.name, oar.modules map { mr => - ModuleReport( - mr.module, - mr.artifacts, - mr.missingArtifacts, - mr.status, - mr.publicationDate, - mr.resolver, - mr.artifactResolver, - mr.evicted, - mr.evictedData, - mr.evictedReason, - mr.problem, - mr.homepage, - mr.extraAttributes, - mr.isDefault, - mr.branch, - mr.configurations, - mr.licenses, - filterOutArtificialCallers(mr.callers) - ) + val callers = filterOutArtificialCallers(mr.callers) + // Reuse the instance when filtering changed nothing, so interning is not undone here. + if (callers eq mr.callers) mr + else + ModuleReport( + mr.module, + mr.artifacts, + mr.missingArtifacts, + mr.status, + mr.publicationDate, + mr.resolver, + mr.artifactResolver, + mr.evicted, + mr.evictedData, + mr.evictedReason, + mr.problem, + mr.homepage, + mr.extraAttributes, + mr.isDefault, + mr.branch, + mr.configurations, + mr.licenses, + callers + ) } ) } diff --git a/lm-core/src/main/scala/sbt/internal/librarymanagement/UpdateReportInterner.scala b/lm-core/src/main/scala/sbt/internal/librarymanagement/UpdateReportInterner.scala new file mode 100644 index 000000000..3da755a08 --- /dev/null +++ b/lm-core/src/main/scala/sbt/internal/librarymanagement/UpdateReportInterner.scala @@ -0,0 +1,74 @@ +/* + * sbt + * Copyright 2023, Scala center + * Copyright 2011 - 2022, Lightbend, Inc. + * Copyright 2008 - 2010, Mark Harrah + * Licensed under Apache License 2.0 (see LICENSE) + */ + +package sbt.internal.librarymanagement + +import java.io.File +import sbt.librarymanagement.* + +/** + * Intern pools for the immutable `UpdateReport` components to save memory. + * + * The pools are weak, so an entry lives only as long as some report references it. + */ +object UpdateReportInterner { + + private val configRefs = new WeakInterner[ConfigRef] + private val rules = new WeakInterner[InclExclRule] + private val artifacts = new WeakInterner[Artifact] + private val moduleIds = new WeakInterner[ModuleID] + private val callers = new WeakInterner[Caller] + private val files = new WeakInterner[File] + private val moduleReports = new WeakInterner[ModuleReport] + + def intern(c: ConfigRef): ConfigRef = configRefs.intern(c) + + def intern(r: InclExclRule): InclExclRule = rules.intern(r) + + def intern(f: File): File = files.intern(f) + + def intern(a: Artifact): Artifact = + artifacts.internWith(a) { artifact => + // Canonicalize the nested vectors so value-equal artifacts share their pieces even when only + // encountered once. + if (artifact.configurations.isEmpty) artifact + else artifact.withConfigurations(artifact.configurations.map(intern)) + } + + def intern(m: ModuleID): ModuleID = + moduleIds.internWith(m) { moduleId => + if ( + moduleId.inclusions.isEmpty && moduleId.exclusions.isEmpty && + moduleId.explicitArtifacts.isEmpty + ) moduleId + else + moduleId + .withInclusions(moduleId.inclusions.map(intern)) + .withExclusions(moduleId.exclusions.map(intern)) + .withExplicitArtifacts(moduleId.explicitArtifacts.map(intern)) + } + + def intern(c: Caller): Caller = + callers.internWith(c) { caller => + caller + .withCaller(intern(caller.caller)) + .withCallerConfigurations(caller.callerConfigurations.map(intern)) + } + + def intern(mr: ModuleReport): ModuleReport = + // A publicationDate is a mutable Calendar, so canonicalize such a report but never pool it. + if (mr.publicationDate.isDefined) canonicalize(mr) + else moduleReports.internWith(mr)(canonicalize) + + private def canonicalize(mr: ModuleReport): ModuleReport = + mr.withModule(intern(mr.module)) + .withArtifacts(mr.artifacts.map { case (a, f) => (intern(a), intern(f)) }) + .withMissingArtifacts(mr.missingArtifacts.map(intern)) + .withConfigurations(mr.configurations.map(intern)) + .withCallers(mr.callers.map(intern)) +} diff --git a/lm-core/src/main/scala/sbt/internal/librarymanagement/UpdateReportPersistence.scala b/lm-core/src/main/scala/sbt/internal/librarymanagement/UpdateReportPersistence.scala index 85d8ba1b6..75d67139b 100644 --- a/lm-core/src/main/scala/sbt/internal/librarymanagement/UpdateReportPersistence.scala +++ b/lm-core/src/main/scala/sbt/internal/librarymanagement/UpdateReportPersistence.scala @@ -26,32 +26,13 @@ final case class UpdateReportCache( object UpdateReportPersistence: /** - * The generated library-management codecs, with the artifact content hash disabled. Persisted update - * reports are the only thing that uses them; everything else keeps the stock `LibraryManagementCodec` - * object, including the `inputs` store, so `Tracked.inputChanged` still hashes contents for - * invalidation. + * The generated library-management codecs, with the artifact content hash disabled: nothing reads the + * hash back, and computing it re-reads the whole downloaded classpath. * - * sjsonnew serializes a `File` as a `(uri, Long)` pair whose Long is - * `HashUtil.sha256ToLong(file.toPath())` -- a full content hash of the file. Nothing reads it back: - * `IsoStringLong[File].from` parses the URI and drops the Long, and `update` decides staleness in - * `LibraryManagement.fileUptodate`, which checks `File.exists` and the modification time against - * `UpdateReport.stamps`. Meanwhile a report names an artifact once per configuration it resolved in, - * and the projects of a build largely share their dependencies, so writing the caches re-reads the - * whole downloaded classpath many times over -- easily the dominant cost of writing them -- to produce - * bytes no reader looks at. - * - * `fileStringLongIso` is an `implicit lazy val` in `sjsonnew.FileIsoStringLongs`, so it is a virtual - * member and every generated format resolves `JsonFormat[File]` as - * `isoStringLongFormat[File](fileStringLongIso)` through its self-type. Overriding it here therefore - * also reaches the `Vector[(Artifact, File)]` nested inside the generated `ModuleReportFormat`, which - * a locally-scoped `JsonFormat[File]` could not. - * - * The JSON shape is unchanged -- only the Long's value is -- so caches stay readable by sbt versions - * that still write the hash, and the ones written here stay readable by them. + * `fileStringLongIso` is virtual, so overriding it also reaches the `Vector[(Artifact, File)]` nested + * inside the generated `ModuleReportFormat`. */ private[sbt] object CacheCodec extends LibraryManagementCodec: - - /** `IO.toURI` emits the same text the stock iso puts in `first`, and `IO.toFile` inverts it. */ override implicit lazy val fileStringLongIso: IsoStringLong[File] = IsoStringLong.iso[File]( (f: File) => (IO.toURI(f).toASCIIString, 0L), @@ -60,10 +41,27 @@ object UpdateReportPersistence: end CacheCodec - // Not the stock `LibraryManagementCodec`: see `CacheCodec` for why persisted reports must not - // content-hash the artifacts they name. import CacheCodec.given + /** Interns the modules of a decoded cache, in a pass since the generated reader has no hook. */ + private def internModules(cache: UpdateReportCache): UpdateReportCache = + cache.copy(lite = + UpdateReportLite( + cache.lite.configurations.map(cr => + ConfigurationReportLite( + cr.configuration, + cr.details.map(d => + OrganizationArtifactReport( + d.organization, + d.name, + d.modules.map(UpdateReportInterner.intern) + ) + ) + ) + ) + ) + ) + given updateReportCacheFormat: JsonFormat[UpdateReportCache] = new JsonFormat[UpdateReportCache]: override def read[J]( @@ -78,7 +76,7 @@ object UpdateReportPersistence: val stamps = unbuilder.readField[Map[String, Long]]("stamps") val cachedDescriptor = unbuilder.readField[File]("cachedDescriptor") unbuilder.endObject() - UpdateReportCache(lite, stats, stamps, cachedDescriptor) + internModules(UpdateReportCache(lite, stats, stamps, cachedDescriptor)) case None => deserializationError("Expected JsObject but found None") @@ -109,6 +107,7 @@ object UpdateReportPersistence: .orElse( Try(store.read[UpdateReport]()).toOption .map(toCache) + .map(internModules) ) def writeTo(store: CacheStore, cache: UpdateReportCache): Unit = diff --git a/lm-core/src/test/scala/sbt/internal/librarymanagement/UpdateReportInternerSpec.scala b/lm-core/src/test/scala/sbt/internal/librarymanagement/UpdateReportInternerSpec.scala new file mode 100644 index 000000000..6e6043fd3 --- /dev/null +++ b/lm-core/src/test/scala/sbt/internal/librarymanagement/UpdateReportInternerSpec.scala @@ -0,0 +1,132 @@ +/* + * sbt + * Copyright 2023, Scala center + * Copyright 2011 - 2022, Lightbend, Inc. + * Copyright 2008 - 2010, Mark Harrah + * Licensed under Apache License 2.0 (see LICENSE) + */ + +package sbt.internal.librarymanagement + +import java.io.File +import java.lang.ref.WeakReference +import java.util.Calendar +import sbt.io.IO +import sbt.librarymanagement.* + +object UpdateReportInternerSpec extends verify.BasicTestSuite: + + test("interning a value-equal ConfigRef returns the same instance"): + val a = ConfigRef("compile") + val b = ConfigRef("compile") + val ia = UpdateReportInterner.intern(a) + assert(ia eq UpdateReportInterner.intern(b), "value-equal ConfigRefs should share one instance") + assert(ia == a, "interning must preserve value equality") + + test("interning a value-equal InclExclRule shares one instance"): + val r1 = InclExclRule().withOrganization("org.bad").withName("bad-lib") + val r2 = InclExclRule().withOrganization("org.bad").withName("bad-lib") + assert(r1 ne r2) + val i1 = UpdateReportInterner.intern(r1) + assert(i1 eq UpdateReportInterner.intern(r2)) + assert(i1 == r1) + + test("interning a value-equal File shares one instance"): + val f1 = new File("/tmp/shared/module.jar") + val f2 = new File("/tmp/shared/module.jar") + assert(f1 ne f2) + assert(UpdateReportInterner.intern(f1) eq UpdateReportInterner.intern(f2)) + + test("interning a value-equal Artifact shares one instance and preserves equality"): + def artifact = + Artifact("lib", "jar", "jar", None, Vector(ConfigRef("compile")), None, Map.empty, None) + val a1 = artifact + val a2 = artifact + assert(a1 ne a2) + val i1 = UpdateReportInterner.intern(a1) + assert(i1 eq UpdateReportInterner.intern(a2)) + assert(i1 == a1) + + test("interning a value-equal ModuleID shares one instance and canonicalizes nested rules"): + def excl = InclExclRule().withOrganization("org.bad").withName("bad") + val m1 = ModuleID("org.example", "lib", "1.0.0").withExclusions(Vector(excl)) + val m2 = ModuleID("org.example", "lib", "1.0.0").withExclusions(Vector(excl)) + assert(m1 ne m2) + val i1 = UpdateReportInterner.intern(m1) + val i2 = UpdateReportInterner.intern(m2) + assert(i1 eq i2, "value-equal ModuleIDs should share one instance") + assert(i1 == m1) + assert(i1.exclusions.head eq i2.exclusions.head, "nested exclusion rules should be shared too") + + test("interning a value-equal Caller shares one instance"): + def caller = + Caller( + ModuleID("org.parent", "parent", "2.0.0"), + Vector(ConfigRef("compile")), + Map.empty, + isForceDependency = false, + isChangingDependency = false, + isTransitiveDependency = true, + isDirectlyForceDependency = false + ) + val c1 = caller + val c2 = caller + assert(c1 ne c2) + assert(UpdateReportInterner.intern(c1) eq UpdateReportInterner.intern(c2)) + + test("interning a value-equal ModuleReport shares one instance"): + IO.withTemporaryDirectory: baseDir => + val jar = new File(baseDir, "pooled.jar") + IO.touch(jar) + def mr = plainModuleReport("org.example", "pooled-lib", jar) + val a = mr + val b = mr + assert(a ne b) + val ia = UpdateReportInterner.intern(a) + assert(ia eq UpdateReportInterner.intern(b), "value-equal reports should share one instance") + assert(ia == a, "pooling must preserve value equality") + + test("a ModuleReport carrying a publicationDate is canonicalized but not pooled"): + IO.withTemporaryDirectory: baseDir => + val jar = new File(baseDir, "dated.jar") + IO.touch(jar) + val epoch = Calendar.getInstance() + epoch.setTimeInMillis(0L) + def mr = plainModuleReport("org.example", "dated-lib", jar).withPublicationDate(Some(epoch)) + val a = UpdateReportInterner.intern(mr) + val b = UpdateReportInterner.intern(mr) + assert(a ne b, "sharing would alias a mutable java.util.Calendar") + assert(a == b, "the two instances must still be value-equal") + assert(a.module eq b.module, "the immutable coordinate inside is still interned") + + test("an interned value is released once nothing else references it"): + // ModuleID has no factory cache and the name is unique, so the pool is the only thing that could + // retain it. A strong pool would pin it for the classloader's life. + val weak = new WeakReference( + UpdateReportInterner.intern(ModuleID("org.example.interner", "released-coordinate", "1.0.0")) + ) + assert(awaitCleared(weak), "the interner must release a value once no report references it") + + test("a pooled ModuleReport is released once nothing else references it"): + IO.withTemporaryDirectory: baseDir => + val jar = new File(baseDir, "transient.jar") + IO.touch(jar) + val weak = new WeakReference( + UpdateReportInterner.intern( + plainModuleReport("org.example.interner", "released-report", jar) + ) + ) + assert(awaitCleared(weak), "the pool must not pin a report no caller references") + + private def plainModuleReport(org: String, name: String, jar: File): ModuleReport = + val artifact = Artifact(name, "jar", "jar", None, Vector.empty, None, Map.empty, None) + ModuleReport(ModuleID(org, name, "1.0.0"), Vector((artifact, jar)), Vector.empty) + .withConfigurations(Vector(ConfigRef("compile"))) + + private def awaitCleared(ref: WeakReference[?]): Boolean = + var i = 0 + while i < 50 && ref.get != null do + System.gc() + Thread.sleep(20) + i += 1 + ref.get == null diff --git a/lm-coursier/src/main/scala/lmcoursier/internal/SbtUpdateReport.scala b/lm-coursier/src/main/scala/lmcoursier/internal/SbtUpdateReport.scala index 83a8936d8..827441c68 100644 --- a/lm-coursier/src/main/scala/lmcoursier/internal/SbtUpdateReport.scala +++ b/lm-coursier/src/main/scala/lmcoursier/internal/SbtUpdateReport.scala @@ -16,6 +16,7 @@ import coursier.core.{ } import coursier.maven.MavenAttributes import coursier.util.Artifact +import sbt.internal.librarymanagement.UpdateReportInterner import sbt.librarymanagement.{ Artifact as _, Configuration as _, * } import sbt.util.Logger import scala.annotation.nowarn @@ -147,23 +148,26 @@ private[internal] object SbtUpdateReport { sbtMissingArtifacts.toVector ) - rep - // .withStatus(None) - .withPublicationDate(publicationDate) - // .withResolver(None) - // .withArtifactResolver(None) - // .withEvicted(false) - // .withEvictedData(None) - // .withEvictedReason(None) - // .withProblem(None) - .withHomepage(Some(project.info.homePage).filter(_.nonEmpty)) - .withLicenses(project.info.licenses.toVector) - .withExtraAttributes(dependency.module.attributes ++ infoProperties(project)) - // .withIsDefault(None) - // .withBranch(None) - .withConfigurations(project.configurations.keys.toVector.map(c => ConfigRef(c.value))) - .withLicenses(project.info.licenses.toVector) - .withCallers(callers.toVector) + // Intern as each report is built, so a coordinate is one instance across every project. + UpdateReportInterner.intern( + rep + // .withStatus(None) + .withPublicationDate(publicationDate) + // .withResolver(None) + // .withArtifactResolver(None) + // .withEvicted(false) + // .withEvictedData(None) + // .withEvictedReason(None) + // .withProblem(None) + .withHomepage(Some(project.info.homePage).filter(_.nonEmpty)) + .withLicenses(project.info.licenses.toVector) + .withExtraAttributes(dependency.module.attributes ++ infoProperties(project)) + // .withIsDefault(None) + // .withBranch(None) + .withConfigurations(project.configurations.keys.toVector.map(c => ConfigRef(c.value))) + .withLicenses(project.info.licenses.toVector) + .withCallers(callers.toVector) + ) } @nowarn diff --git a/main/src/main/scala/sbt/internal/LibraryManagement.scala b/main/src/main/scala/sbt/internal/LibraryManagement.scala index 73e5f3295..935677cd9 100644 --- a/main/src/main/scala/sbt/internal/LibraryManagement.scala +++ b/main/src/main/scala/sbt/internal/LibraryManagement.scala @@ -265,7 +265,12 @@ private[sbt] object LibraryManagement { else crs1 map { cr => val mrs0 = cr.modules - val mrs1 = mrs0 map { _.withCallers(Vector()) } + // Re-intern what stripping rebuilds: dropping the callers is what makes two projects' + // reports of the same coordinate value-equal, so this is where they collapse to one. + val mrs1 = mrs0 map { mr => + if (mr.callers.isEmpty) mr + else UpdateReportInterner.intern(mr.withCallers(Vector())) + } cr.withModules(mrs1) } ur.withConfigurations(crs2) diff --git a/sbt-app/src/sbt-test/dependency-management/update-report-interning/build.sbt b/sbt-app/src/sbt-test/dependency-management/update-report-interning/build.sbt new file mode 100644 index 000000000..44e03b315 --- /dev/null +++ b/sbt-app/src/sbt-test/dependency-management/update-report-interning/build.sbt @@ -0,0 +1,43 @@ +ThisBuild / scalaVersion := "2.13.16" + +lazy val checkSharedAcrossProjects = taskKey[Unit]("Validates cross-project module report sharing") + +// Two projects resolving the same coordinate independently. Coursier memoizes moduleReport on a key +// that includes the dependees, so each project builds its own instance; they become value-equal only +// once `update` strips the callers, which is where interning collapses them. +lazy val a = project.settings( + libraryDependencies += "org.scala-lang.modules" %% "scala-xml" % "2.4.0" +) + +lazy val b = project.settings( + libraryDependencies += "org.scala-lang.modules" %% "scala-xml" % "2.4.0" +) + +lazy val root = (project in file(".")) + .aggregate(a, b) + .settings( + checkSharedAcrossProjects := { + def xml(ur: UpdateReport) = + ur.configurations + .find(_.configuration.name == "compile") + .getOrElse(sys.error("no compile configuration")) + .modules + .find(_.module.name.startsWith("scala-xml")) + .getOrElse(sys.error("scala-xml not in the report")) + val fromA = xml((a / update).value) + val fromB = xml((b / update).value) + require(fromA == fromB, s"expected value-equal reports, got\n$fromA\nand\n$fromB") + require( + fromA.callers.isEmpty, + s"update should have stripped the callers, got ${fromA.callers}" + ) + require( + fromA eq fromB, + "two projects resolving one coordinate must share a single ModuleReport instance" + ) + require( + fromA.module eq fromB.module, + "the ModuleID inside must be shared too" + ) + } + ) diff --git a/sbt-app/src/sbt-test/dependency-management/update-report-interning/test b/sbt-app/src/sbt-test/dependency-management/update-report-interning/test new file mode 100644 index 000000000..3cb072a6e --- /dev/null +++ b/sbt-app/src/sbt-test/dependency-management/update-report-interning/test @@ -0,0 +1,6 @@ +# Both projects resolve fresh here, so this exercises the coursier construction path +# rather than the cached-read path. +> checkSharedAcrossProjects + +# And again with the reports coming back from the cache. +> checkSharedAcrossProjects