[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) <noreply@anthropic.com>

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

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

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 }

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"
+  )
+}

sbt-app/src/sbt-test/cache/analysis-cache-reads/changes/A2.scala

@@ -0,0 +1 @@
+object A { def a: Int = L.l + 1 }

sbt-app/src/sbt-test/cache/analysis-cache-reads/lib/src/main/scala/L.scala

@@ -0,0 +1 @@
+object L { def l: Int = 1 }

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)

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