From 0ac15531a8cbbea89503b823e30d769d88f6f298 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mai=20Huy=20Ho=C3=A0ng?= Date: Tue, 11 Aug 2026 02:57:18 +0700 Subject: [PATCH] [2.x] perf: Keep the analysis cached across an action cache output sync (#9550) The local analysis cache validates an entry against the analysis file's timestamp and size, so a file re-created with the same content loses its entry. That is exactly what happens to every analysis sbt writes: the analysis file is a declared output of compileIncremental, so once the task body has written it, syncFile deletes it and re-creates it as a symlink into the CAS. Measured on a two-module build, the file came back four milliseconds later at the same size under a new timestamp, and the read in compileIncremental that follows deserialized the analysis it had just written - one full deserialization per compiled module per compile, which is the cost the cache exists to avoid. On this repo's own largest analysis (519K) that read costs 19.5ms, against 0.098ms to hash the file and 0.0025ms to stat it. Populating the cache from set() is not enough on its own, and populating it with the contents handed to set() is wrong: ConsistentAnalysisFormat does not persist Compilations, so serving the in-memory analysis flips CompileResult.hasModified from false to true and makes compileTask store the analysis on every compile, including no-ops. set() now records what a read of the file returns, and an entry whose size and content hash still match is served under a new timestamp rather than discarded. A read whose timestamp and size still match does not hash the file, so a warm no-op compile costs one stat per read; a read that finds no entry to compare against hashes the file it is about to deserialize, and set() hashes the file it wrote, since the file it has to describe is replaced moments later. Recording an entry cannot fail a compile that has already written its analysis, so an IO error from stat-ing or hashing that file is dropped. Across five compiles of two modules with two source edits, the deserializations go from five to none: previousCompile, compileIncremental, compileScalaBackend and the dependency analysis read in compileIncSetup are all served from memory after the first write of each module's analysis. Nothing a build can observe says whether a compile deserialized an analysis it already had: an analysis served from the cache is indistinguishable from one read back, so the unit tests can pin the store's behaviour but not the compile's. The cache therefore counts what its reads cost - answered from memory, hashed the file to answer, deserialized it - and a scripted test reads those counts around a compile. The counts are sbt-private, so the test reaches them from a helper declared in package sbt under project/. Two invariants, one per failure mode. A compile with nothing to do must answer every read from memory without even re-reading the file to hash it, which pins the timestamp check; dropping it makes 7 of 7 reads hash. A compile that recompiles must be served the analysis it just wrote, which pins the fix; keeping develop's cache and adding only the counters deserializes 1 of 7. Both depend on the compiles being real ones, because an analysis the action cache served was never written and so there is nothing for the local cache to have kept. The action cache is global, and its key does not depend on where the build sits, so a second run of this test would otherwise recompile nothing: the build points localCacheDirectory inside the sandbox - in Global, which is the scope cacheStores resolves it in - and the test deletes target first, so a run cannot inherit what an earlier one compiled. Generated-by: claude-opus-5 (Claude Code) Co-authored-by: Claude Opus 5 (1M context) --- .../main/scala/sbt/internal/BuildDef.scala | 100 ++++++++-- .../sbt/internal/LocalAnalysisCacheTest.scala | 171 ++++++++++++++++++ .../app/src/main/scala/A.scala | 1 + .../cache/analysis-cache-reads/build.sbt | 44 +++++ .../analysis-cache-reads/changes/A2.scala | 1 + .../lib/src/main/scala/L.scala | 1 + .../project/AnalysisCacheReads.scala | 11 ++ .../sbt-test/cache/analysis-cache-reads/test | 24 +++ 8 files changed, 340 insertions(+), 13 deletions(-) create mode 100644 main/src/test/scala/sbt/internal/LocalAnalysisCacheTest.scala create mode 100644 sbt-app/src/sbt-test/cache/analysis-cache-reads/app/src/main/scala/A.scala create mode 100644 sbt-app/src/sbt-test/cache/analysis-cache-reads/build.sbt create mode 100644 sbt-app/src/sbt-test/cache/analysis-cache-reads/changes/A2.scala create mode 100644 sbt-app/src/sbt-test/cache/analysis-cache-reads/lib/src/main/scala/L.scala create mode 100644 sbt-app/src/sbt-test/cache/analysis-cache-reads/project/AnalysisCacheReads.scala create mode 100644 sbt-app/src/sbt-test/cache/analysis-cache-reads/test diff --git a/main/src/main/scala/sbt/internal/BuildDef.scala b/main/src/main/scala/sbt/internal/BuildDef.scala index 540c09530..e9cd533a8 100644 --- a/main/src/main/scala/sbt/internal/BuildDef.scala +++ b/main/src/main/scala/sbt/internal/BuildDef.scala @@ -10,16 +10,24 @@ package sbt package internal import com.github.benmanes.caffeine.cache.{ Cache as CCache, Caffeine, Weigher } -import java.io.File +import java.io.{ File, IOException } import java.nio.file.{ Files, NoSuchFileException, Path as NioPath } import java.nio.file.attribute.BasicFileAttributes import java.util.Optional +import java.util.concurrent.atomic.LongAdder import Keys.{ organization, thisProject, autoGeneratedProject, publish, publishLocal, skip } import Def.Setting // import sbt.ProjectExtra.apply import sbt.io.Hash import sbt.internal.util.{ Attributed, StringAttributeMap } -import sbt.internal.inc.{ FileAnalysisStore, MixedAnalyzingCompiler, ReflectUtilities } +import sbt.internal.inc.{ + Analysis, + Compilations, + FileAnalysisStore, + HashUtil, + MixedAnalyzingCompiler, + ReflectUtilities +} import sbt.util.CacheImplicits.given import scala.jdk.OptionConverters.* import xsbti.{ FileConverter, VirtualFileRef } @@ -93,24 +101,64 @@ private[sbt] object BuildDef: in.flatMap(a => extractAnalysis(a.metadata, converter)) private[sbt] final val localAnalysisCacheByteSize = 100 * 1024L * 1024L - private val weigher: Weigher[String, (Option[AnalysisContents], Long, Long)] = { - case (_, (_, _, sizeBytes)) => sizeBytes.toInt + + /** The analysis a file holds, with what it takes to tell that the file still holds it. */ + private final class CachedAnalysis( + val contents: Option[AnalysisContents], + val lastModified: Long, + val sizeBytes: Long, + val contentHash: Long, + ) + private val weigher: Weigher[String, CachedAnalysis] = { case (_, cached) => + cached.sizeBytes.toInt } - private val inMemoryAnalysisCache: CCache[String, (Option[AnalysisContents], Long, Long)] = + private val inMemoryAnalysisCache: CCache[String, CachedAnalysis] = Caffeine .newBuilder() .maximumWeight(localAnalysisCacheByteSize) .weigher(weigher) .build() - private def getOrElseUpdate(ref: VirtualFileRef, lastModified: Long, sizeBytes: Long)( + private def getOrElseUpdate( + ref: VirtualFileRef, + path: NioPath, + lastModified: Long, + sizeBytes: Long + )( value: => Option[AnalysisContents] ): Option[AnalysisContents] = + lazy val contentHash = + AnalysisCacheStats.hashed() + HashUtil.farmHash(path) + + def record(contents: Option[AnalysisContents], hash: Long): Option[AnalysisContents] = + inMemoryAnalysisCache.put(ref.id(), CachedAnalysis(contents, lastModified, sizeBytes, hash)) + contents + Option(inMemoryAnalysisCache.getIfPresent(ref.id())) match - case Some((v, mod, i)) if lastModified == mod && sizeBytes == i => v - case _ => - val v = value - inMemoryAnalysisCache.put(ref.id(), (v, lastModified, sizeBytes)) - v + case Some(cached) if cached.lastModified == lastModified && cached.sizeBytes == sizeBytes => + AnalysisCacheStats.served() + cached.contents + // an action cache output is re-created in place once it has been written, which brings the + // same analysis back under a new timestamp + case Some(cached) if cached.sizeBytes == sizeBytes && cached.contentHash == contentHash => + AnalysisCacheStats.served() + record(cached.contents, cached.contentHash) + case _ => + AnalysisCacheStats.deserialized() + record(value, contentHash) + + private[sbt] object AnalysisCacheStats: + private val servedCount = new LongAdder + private val hashedCount = new LongAdder + private val deserializedCount = new LongAdder + + def served(): Unit = servedCount.increment() + def hashed(): Unit = hashedCount.increment() + def deserialized(): Unit = deserializedCount.increment() + + def reads: (Long, Long, Long) = + (servedCount.sum(), hashedCount.sum(), deserializedCount.sum()) + end AnalysisCacheStats private[sbt] def extractAnalysis( metadata: StringAttributeMap, @@ -129,13 +177,23 @@ private[sbt] object BuildDef: else val lastModified = attrs.lastModifiedTime().toMillis() val sizeBytes = attrs.size() - getOrElseUpdate(ref, lastModified, sizeBytes)(fallback(file)) + getOrElseUpdate(ref, path, lastModified, sizeBytes)(fallback(file)) catch case _: NoSuchFileException => fallback(file) for ref <- metadata.get(Keys.analysis) content <- getContents(VirtualFileRef.of(ref)) yield content.getAnalysis + /** + * The binary format does not persist [[Compilations]], and `compileScalaBackend` derives + * `hasModified` from them, so a stored analysis is cached without its own. + */ + private def asPersisted(contents: AnalysisContents): AnalysisContents = + contents.getAnalysis match + case a: Analysis if a.compilations.allCompilations.nonEmpty => + AnalysisContents.create(a.copy(compilations = Compilations.empty), contents.getMiniSetup) + case _ => contents + private[sbt] def cachedAnalysisStore(path: NioPath, converter: FileConverter): AnalysisStore = CachedAnalysisStore(path, converter) @@ -153,7 +211,7 @@ private[sbt] object BuildDef: else val lastModified = attrs.lastModifiedTime().toMillis() val sizeBytes = attrs.size() - getOrElseUpdate(ref, lastModified, sizeBytes)(underlying.get.toScala).toJava + getOrElseUpdate(ref, path, lastModified, sizeBytes)(underlying.get.toScala).toJava catch case _: NoSuchFileException => underlying.get override def unsafeGet: AnalysisContents = get.toScala.get @@ -162,6 +220,22 @@ private[sbt] object BuildDef: val ref: VirtualFileRef = converter.toVirtualFile(path) inMemoryAnalysisCache.invalidate(ref.id()) underlying.set(contents) + // the attributes have to be the written file's, not the ones it had before the write, which + // assumes this path has no other writer: one compile per project and configuration + try + val attrs = Files.readAttributes(path, classOf[BasicFileAttributes]) + if !attrs.isDirectory then + inMemoryAnalysisCache.put( + ref.id(), + CachedAnalysis( + Some(asPersisted(contents)), + attrs.lastModifiedTime().toMillis(), + attrs.size(), + HashUtil.farmHash(path), + ) + ) + // the analysis is on disk either way, so nothing about caching it may fail the compile + catch case _: IOException => () end CachedAnalysisStore end BuildDef diff --git a/main/src/test/scala/sbt/internal/LocalAnalysisCacheTest.scala b/main/src/test/scala/sbt/internal/LocalAnalysisCacheTest.scala new file mode 100644 index 000000000..00c34c006 --- /dev/null +++ b/main/src/test/scala/sbt/internal/LocalAnalysisCacheTest.scala @@ -0,0 +1,171 @@ +/* + * 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 +package internal + +import hedgehog.* +import hedgehog.runner.* +import java.nio.file.{ Files, Path } +import java.nio.file.attribute.FileTime +import _root_.sbt.internal.inc.{ + Analysis, + Compilation, + Compilations, + CompileOutput, + FarmHash, + FileAnalysisStore, + MappedFileConverter, + SourceInfos +} +import _root_.sbt.internal.inc.Analysis.NonLocalProduct +import _root_.sbt.io.IO +import _root_.sbt.io.syntax.* +import _root_.sbt.util.InterfaceUtil.t2 +import scala.jdk.OptionConverters.* +import xsbti.{ FileConverter, VirtualFileRef } +import xsbti.compile.{ + AnalysisContents, + AnalysisStore, + CompileOrder, + FileHash, + MiniOptions, + MiniSetup +} + +object LocalAnalysisCacheTest extends Properties: + override def tests: List[Test] = List( + example("a stored analysis is served from the local cache", storedAnalysisIsServed), + example("a re-created analysis file stays cached", reCreatedFileStaysCached), + example("a rewritten analysis file is read again", rewrittenFileIsReadAgain), + example("the compilations a read drops are not served", compilationsAreNotServed), + ) + + /** + * `compileIncremental` stores the analysis it just computed, and the next reader goes through a + * store of its own, so the value has to come from the process-wide cache rather than from a field + * of the instance that wrote it. + */ + def storedAnalysisIsServed: Result = + withAnalysisFile: file => + val contents = contentsOf(oneSourceAnalysis) + cachedStore(file).set(contents) + val got = cachedStore(file).get().toScala + Result + .assert(got.exists(_ eq contents)) + .log("expected the stored analysis, not a re-read of the file") + + /** + * The analysis file is a declared output of `compileIncremental`, so the action cache re-creates + * it in place right after it is written: same content under a new timestamp. + */ + def reCreatedFileStaysCached: Result = + withAnalysisFile: file => + val contents = contentsOf(oneSourceAnalysis) + val store = cachedStore(file) + store.set(contents) + reCreate(file) + val got = store.get().toScala + Result + .assert(got.exists(_ eq contents)) + .log("re-creating the file with the same content must not discard the cached analysis") + + /** A file the action cache switches to another analysis holds that other analysis. */ + def rewrittenFileIsReadAgain: Result = + withAnalysisFile: file => + val contents = contentsOf(oneSourceAnalysis) + val store = cachedStore(file) + store.set(contents) + FileAnalysisStore.binary(file.toFile).set(contentsOf(Analysis.empty)) + val got = store.get().toScala + Result.all( + List( + Result + .assert(!got.exists(_ eq contents)) + .log("must not serve the analysis the file no longer holds"), + Result + .assert(got.exists(_.getAnalysis.readStamps.getAllSourceStamps.isEmpty)) + .log("expected the analysis now on disk"), + ) + ) + + /** + * The binary format does not persist compilations, and `compileScalaBackend` reads them back to + * decide whether the compile modified anything, so a stored analysis may not carry its own. + */ + def compilationsAreNotServed: Result = + withAnalysisFile: file => + val compiled = oneSourceAnalysis.copy( + compilations = Compilations.empty.add(Compilation(1L, setup.output)) + ) + val store = cachedStore(file) + store.set(contentsOf(compiled)) + val fromCache = store.get().toScala + inMemoryStore(file).set(contentsOf(compiled)) + val fromFile = inMemoryStore(file).get().toScala + Result.all( + List( + Result + .assert(fromCache.exists(_.getAnalysis.readCompilations.getAllCompilations.isEmpty)) + .log("a stored analysis must be served as the file holds it"), + Result + .assert(fromFile.exists(_.getAnalysis.readCompilations.getAllCompilations.isEmpty)) + .log("expected the file itself to hold no compilations"), + ) + ) + + // ---------- helpers ---------- + + private val converter: FileConverter = MappedFileConverter.empty + + private def cachedStore(file: Path): AnalysisStore = + BuildDef.cachedAnalysisStore(file, converter) + + private def inMemoryStore(file: Path): AnalysisStore = + FileAnalysisStore.binary(file.toFile) + + /** Replaces `file` with its own content, as syncing an action cache output does. */ + private def reCreate(file: Path): Unit = + val content = Files.readAllBytes(file) + val lastModified = Files.getLastModifiedTime(file).toMillis + IO.delete(file.toFile) + Files.write(file, content) + Files.setLastModifiedTime(file, FileTime.fromMillis(lastModified + 5000)) + + private def withAnalysisFile(f: Path => Result): Result = + IO.withTemporaryDirectory: tmp => + f((tmp / "inc_compile.zip").toPath) + + private val setup: MiniSetup = + MiniSetup.of( + CompileOutput((file("target") / "classes").toPath), + MiniOptions.of(Array.empty[FileHash], Array.empty[String], Array.empty[String]), + "3.3.1", + CompileOrder.Mixed, + true, + Array(t2("key" -> "value")), + ) + + private def contentsOf(analysis: Analysis): AnalysisContents = + AnalysisContents.create(analysis, setup) + + private def oneSourceAnalysis: Analysis = + val stamp = FarmHash.fromLong(1L) + Analysis.empty.addSource( + src = VirtualFileRef.of("A.scala"), + apis = Nil, + stamp = stamp, + info = SourceInfos.emptyInfo, + nonLocalProducts = NonLocalProduct("A", "A", VirtualFileRef.of("A.class"), stamp) :: Nil, + localProducts = Nil, + internalDeps = Nil, + externalDeps = Nil, + libraryDeps = (VirtualFileRef.of("x.jar"), "x", stamp) :: Nil, + ) + +end LocalAnalysisCacheTest diff --git a/sbt-app/src/sbt-test/cache/analysis-cache-reads/app/src/main/scala/A.scala b/sbt-app/src/sbt-test/cache/analysis-cache-reads/app/src/main/scala/A.scala new file mode 100644 index 000000000..1ad3fb50d --- /dev/null +++ b/sbt-app/src/sbt-test/cache/analysis-cache-reads/app/src/main/scala/A.scala @@ -0,0 +1 @@ +object A { def a: Int = L.l } diff --git a/sbt-app/src/sbt-test/cache/analysis-cache-reads/build.sbt b/sbt-app/src/sbt-test/cache/analysis-cache-reads/build.sbt new file mode 100644 index 000000000..12166479a --- /dev/null +++ b/sbt-app/src/sbt-test/cache/analysis-cache-reads/build.sbt @@ -0,0 +1,44 @@ +ThisBuild / scalaVersion := "3.3.6" + +// The compiles below have to be real ones: an analysis served from the action cache was never +// written, so there is nothing for the cache to have kept. Keeping the cache inside the sandbox +// means each run starts without one, rather than inheriting whatever an earlier run compiled. +// Global, because that is the scope cacheStores resolves it in. +Global / localCacheDirectory := (ThisBuild / baseDirectory).value / "target" / "local-cache" + +lazy val lib = project +lazy val app = project.dependsOn(lib) + +val recordAnalysisReads = taskKey[Unit]("Marks the analysis cache read counts.") +val checkAnalysisServedFromMemory = + taskKey[Unit]("Fails if any analysis was read from disk since the mark.") +val checkAnalysisNotDeserialized = + taskKey[Unit]("Fails if any analysis was deserialized since the mark.") + +recordAnalysisReads := Def.uncached(AnalysisCacheReads.record()) + +checkAnalysisServedFromMemory := Def.uncached { + val (served, hashed, deserialized) = AnalysisCacheReads.since() + assert( + served > 0, + s"expected the compile to read an analysis, but it read none" + ) + assert( + hashed == 0 && deserialized == 0, + s"expected every analysis read to be answered from memory, " + + s"but $hashed hashed the file and $deserialized deserialized it" + ) +} + +checkAnalysisNotDeserialized := Def.uncached { + val (served, hashed, deserialized) = AnalysisCacheReads.since() + assert( + served > 0, + s"expected the compile to read an analysis, but it read none" + ) + assert( + deserialized == 0, + s"expected the analysis just written to be served from memory, " + + s"but $deserialized of ${served + deserialized} reads deserialized it" + ) +} diff --git a/sbt-app/src/sbt-test/cache/analysis-cache-reads/changes/A2.scala b/sbt-app/src/sbt-test/cache/analysis-cache-reads/changes/A2.scala new file mode 100644 index 000000000..e45edab14 --- /dev/null +++ b/sbt-app/src/sbt-test/cache/analysis-cache-reads/changes/A2.scala @@ -0,0 +1 @@ +object A { def a: Int = L.l + 1 } diff --git a/sbt-app/src/sbt-test/cache/analysis-cache-reads/lib/src/main/scala/L.scala b/sbt-app/src/sbt-test/cache/analysis-cache-reads/lib/src/main/scala/L.scala new file mode 100644 index 000000000..7b890fa08 --- /dev/null +++ b/sbt-app/src/sbt-test/cache/analysis-cache-reads/lib/src/main/scala/L.scala @@ -0,0 +1 @@ +object L { def l: Int = 1 } diff --git a/sbt-app/src/sbt-test/cache/analysis-cache-reads/project/AnalysisCacheReads.scala b/sbt-app/src/sbt-test/cache/analysis-cache-reads/project/AnalysisCacheReads.scala new file mode 100644 index 000000000..2205b775a --- /dev/null +++ b/sbt-app/src/sbt-test/cache/analysis-cache-reads/project/AnalysisCacheReads.scala @@ -0,0 +1,11 @@ +// In package sbt so that the read counts, which are sbt-private, are reachable from this build. +package sbt + +object AnalysisCacheReads: + @volatile private var mark: (Long, Long, Long) = (0L, 0L, 0L) + + def record(): Unit = mark = sbt.internal.BuildDef.AnalysisCacheStats.reads + + def since(): (Long, Long, Long) = + val (served, hashed, deserialized) = sbt.internal.BuildDef.AnalysisCacheStats.reads + (served - mark._1, hashed - mark._2, deserialized - mark._3) diff --git a/sbt-app/src/sbt-test/cache/analysis-cache-reads/test b/sbt-app/src/sbt-test/cache/analysis-cache-reads/test new file mode 100644 index 000000000..b23b0f083 --- /dev/null +++ b/sbt-app/src/sbt-test/cache/analysis-cache-reads/test @@ -0,0 +1,24 @@ +# The local analysis cache exists so that a compile does not deserialize an analysis it already +# has in memory. Nothing a build can observe says whether it did, so this reads the cache's own +# read counts. + +# An analysis served from the action cache was never written, so there would be nothing for the +# local cache to have kept. Starting without a cache is what makes the compiles below real ones. +$ delete target + +# First compile writes each module's analysis, which is what later reads must be served from. +> app/compile + +# A compile with nothing to do must answer every analysis read from memory, without even +# re-reading the file to hash it. +> recordAnalysisReads +> app/compile +> checkAnalysisServedFromMemory + +# A compile that recompiles writes the analysis and then reads it back. The action cache +# re-creates the file it wrote, under a new timestamp, so the read has to hash the file - but it +# must still be served from memory rather than deserialized again. +> recordAnalysisReads +$ copy-file changes/A2.scala app/src/main/scala/A.scala +> app/compile +> checkAnalysisNotDeserialized