[2.x] fix: Preserve file timestamp in local diskcache (#9559)

**Problem**
In theory, we shouldn't have to care about the timestamps,
but for local optimization it's still a useful key for caching.
Currently local Analysis cache gets invalidated after
syncBlob replaces the Analysis file with a symlink.

**Solution**
This preserves the original timestamp when copying files to
the local diskcache.
This commit is contained in:
eugene yokota 2026-08-06 20:51:23 -04:00 committed by GitHub
parent 1038618246
commit b109068711
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 73 additions and 25 deletions

View File

@ -215,7 +215,7 @@ case class DiskActionCacheStore(base: Path, converter: FileConverter)
val inlineRefs = request.inlineOutputFiles.map: path => val inlineRefs = request.inlineOutputFiles.map: path =>
value.outputFiles.find(_.id == path).get value.outputFiles.find(_.id == path).get
val contents = getBlobs(inlineRefs).toVector.map: b => val contents = getBlobs(inlineRefs).toVector.map: b =>
ByteBuffer.wrap(IO.readBytes(b.input)) Using.resource(b.input)(in => ByteBuffer.wrap(IO.readBytes(in)))
Right(value.withContents(contents)) Right(value.withContents(contents))
catch case NonFatal(e) => Left(e) catch case NonFatal(e) => Left(e)
else Left(notFound) else Left(notFound)
@ -231,17 +231,23 @@ case class DiskActionCacheStore(base: Path, converter: FileConverter)
catch case e: IOException => Left(e) catch case e: IOException => Left(e)
override def putBlobs(blobs: Seq[VirtualFile]): Seq[HashedVirtualFileRef] = override def putBlobs(blobs: Seq[VirtualFile]): Seq[HashedVirtualFileRef] =
blobs.map: (b: VirtualFile) => blobs.map:
putBlob(b.input, Digest(b)) case b: PathBasedFile =>
(b: HashedVirtualFileRef) putBlob(b.toPath(), Digest(b))
(b: HashedVirtualFileRef)
case b: VirtualFile =>
Using.resource(b.input)(putBlob(_, Digest(b)))
(b: HashedVirtualFileRef)
def toCasFile(digest: Digest): Path = def toCasFile(digest: Digest): Path =
(casBase.toFile / digest.toString.replace("/", "-")).toPath() (casBase.toFile / digest.toString.replace("/", "-")).toPath()
def putBlob(blob: Path, digest: Digest): Path = def putBlob(blob: Path, digest: Digest): Path =
Using.resource(Files.newInputStream(blob)) { in => val casFile = toCasFile(digest)
putBlob(in, digest) if isCompleteBlob(casFile, digest) then casFile
} else
IO.copyFile(blob.toFile(), casFile.toFile(), preserveLastModified = true)
casFile
/** Move blob directly to CAS. Internal use only. */ /** Move blob directly to CAS. Internal use only. */
private[sbt] def putBlobInternal(blob: Path, digest: Digest): Path = private[sbt] def putBlobInternal(blob: Path, digest: Digest): Path =

View File

@ -21,14 +21,16 @@ end CacheImplicits
trait CacheImplicits extends BasicCacheImplicits with BasicJsonProtocol: trait CacheImplicits extends BasicCacheImplicits with BasicJsonProtocol:
private val localDigestCacheByteSize = AtomicLong(CacheImplicits.defaultLocalDigestCacheByteSize) private val localDigestCacheByteSize = AtomicLong(CacheImplicits.defaultLocalDigestCacheByteSize)
private val weigher: Weigher[String, (String, Long, Long)] = { case (k, (v1, _, _)) => private val weigher: Weigher[String, (String, Long, Long, Option[AnyRef])] = {
k.size + v1.size + 16 case (k, (v1, _, _, _)) =>
k.size + v1.size + 16
} }
private val digestWeigher: Weigher[String, (Digest, Long, Long)] = { case (k, (v1, _, _)) => private val digestWeigher: Weigher[String, (Digest, Long, Long, Option[AnyRef])] = {
k.size + v1.digestSize + 16 case (k, (v1, _, _, _)) =>
k.size + v1.digestSize + 16
} }
private val stampCache: AtomicReference[CCache[String, (String, Long, Long)]] = private val stampCache: AtomicReference[CCache[String, (String, Long, Long, Option[AnyRef])]] =
AtomicReference( AtomicReference(
Caffeine Caffeine
.newBuilder() .newBuilder()
@ -37,7 +39,7 @@ trait CacheImplicits extends BasicCacheImplicits with BasicJsonProtocol:
.build() .build()
) )
private val digestCache: AtomicReference[CCache[String, (Digest, Long, Long)]] = private val digestCache: AtomicReference[CCache[String, (Digest, Long, Long, Option[AnyRef])]] =
AtomicReference( AtomicReference(
Caffeine Caffeine
.newBuilder() .newBuilder()
@ -67,24 +69,36 @@ trait CacheImplicits extends BasicCacheImplicits with BasicJsonProtocol:
.build() .build()
) )
private def getOrElseUpdate(ref: HashedVirtualFileRef, lastModified: Long, sizeBytes: Long)( // `fileKey` is the path's identity on disk (e.g. device+inode on POSIX), read alongside
// lastModified/sizeBytes from the same attributes call.
private def getOrElseUpdate(
ref: HashedVirtualFileRef,
lastModified: Long,
sizeBytes: Long,
fileKey: Option[AnyRef]
)(
value: => String value: => String
) = ) =
Option(stampCache.get().getIfPresent(ref.id())) match Option(stampCache.get().getIfPresent(ref.id())) match
case Some((v, mod, i)) if lastModified == mod && sizeBytes == i => v case Some((v, mod, i, fk)) if lastModified == mod && sizeBytes == i && fk == fileKey => v
case _ => case _ =>
val v = value val v = value
stampCache.get().put(ref.id(), (v, lastModified, sizeBytes)) stampCache.get().put(ref.id(), (v, lastModified, sizeBytes, fileKey))
v v
private def getOrElseUpdate(ref: VirtualFileRef, lastModified: Long, sizeBytes: Long)( private def getOrElseUpdate(
ref: VirtualFileRef,
lastModified: Long,
sizeBytes: Long,
fileKey: Option[AnyRef]
)(
value: => Digest value: => Digest
) = ) =
Option(digestCache.get().getIfPresent(ref.id())) match Option(digestCache.get().getIfPresent(ref.id())) match
case Some((v, mod, i)) if lastModified == mod && sizeBytes == i => v case Some((v, mod, i, fk)) if lastModified == mod && sizeBytes == i && fk == fileKey => v
case _ => case _ =>
val v = value val v = value
digestCache.get().put(ref.id(), (v, lastModified, sizeBytes)) digestCache.get().put(ref.id(), (v, lastModified, sizeBytes, fileKey))
v v
/** /**
@ -103,7 +117,8 @@ trait CacheImplicits extends BasicCacheImplicits with BasicJsonProtocol:
else else
val lastModified = attrs.lastModifiedTime().toMillis() val lastModified = attrs.lastModifiedTime().toMillis()
val sizeBytes = attrs.size() val sizeBytes = attrs.size()
getOrElseUpdate(ref, lastModified, sizeBytes)(fallback) val fileKey = Option(attrs.fileKey())
getOrElseUpdate(ref, lastModified, sizeBytes, fileKey)(fallback)
catch case e: NoSuchFileException => throw e catch case e: NoSuchFileException => throw e
case _ => fallback case _ => fallback
@ -117,10 +132,11 @@ trait CacheImplicits extends BasicCacheImplicits with BasicJsonProtocol:
else else
val lastModified = attrs.lastModifiedTime().toMillis() val lastModified = attrs.lastModifiedTime().toMillis()
val sizeBytes = attrs.size() val sizeBytes = attrs.size()
val fileKey = Option(attrs.fileKey())
vf match vf match
case h: HashedVirtualFileRef => case h: HashedVirtualFileRef =>
getOrElseUpdate(vf, lastModified, sizeBytes)(Digest(h)) getOrElseUpdate(vf, lastModified, sizeBytes, fileKey)(Digest(h))
case _ => case _ =>
getOrElseUpdate(vf, lastModified, sizeBytes)(fallback) getOrElseUpdate(vf, lastModified, sizeBytes, fileKey)(fallback)
case _ => Digest.sha256Hash(converter.toPath(vf)) case _ => Digest.sha256Hash(converter.toPath(vf))
end CacheImplicits end CacheImplicits

View File

@ -15,6 +15,7 @@ import xsbti.{
CompileFailed, CompileFailed,
FileConverter, FileConverter,
HashedVirtualFileRef, HashedVirtualFileRef,
PathBasedFile,
Problem, Problem,
Position, Position,
Severity, Severity,
@ -425,6 +426,29 @@ object ActionCacheTest extends BasicTestSuite:
"stored ref must not re-stat the file" "stored ref must not re-stat the file"
) )
test("Restore preserves the last-modified time through the CAS round-trip"):
withDiskCache: cache =>
import sjsonnew.BasicJsonProtocol.*
IO.withTemporaryDirectory: tempDir =>
val outPath = (tempDir / "a.txt").toPath
val midnightUTC = java.nio.file.attribute.FileTime.from(
java.time.Instant.parse("2026-08-01T00:00:00Z")
)
val action: ((Int, Int)) => InternalActionResult[Int] = { (a, b) =>
Files.writeString(outPath, "foo")
Files.setLastModifiedTime(outPath, midnightUTC)
InternalActionResult(a + b, Seq(binaryConverter.toVirtualFile(outPath)))
}
val config = getCacheConfig(cache, tempDir, converter = binaryConverter)
val v1 = ActionCache.cache((1, 1), Digest.zero, Digest.zero, tags, config)(action)
assert(v1 == 2)
val actual = Files.getLastModifiedTime(outPath)
assert(
actual == midnightUTC,
s"expected the file's mtime to survive the CAS round-trip unchanged " +
s"(wrote $midnightUTC, but after caching it reads $actual)"
)
test("A successful task whose value fails to serialize returns it uncached"): test("A successful task whose value fails to serialize returns it uncached"):
withDiskCache: cache => withDiskCache: cache =>
var called = 0 var called = 0
@ -746,7 +770,8 @@ object ActionCacheTest extends BasicTestSuite:
final class DiskVirtualFile(path: String) final class DiskVirtualFile(path: String)
extends xsbti.BasicVirtualFileRef(path) extends xsbti.BasicVirtualFileRef(path)
with VirtualFile: with VirtualFile
with PathBasedFile:
private def bytes: Array[Byte] = private def bytes: Array[Byte] =
if Files.isRegularFile(Paths.get(path)) then Files.readAllBytes(Paths.get(path)) if Files.isRegularFile(Paths.get(path)) then Files.readAllBytes(Paths.get(path))
else Array.emptyByteArray else Array.emptyByteArray
@ -754,4 +779,5 @@ object ActionCacheTest extends BasicTestSuite:
override def sizeBytes: Long = bytes.length.toLong override def sizeBytes: Long = bytes.length.toLong
override def contentHashStr: String = Digest.sha256Hash(bytes).contentHashStr override def contentHashStr: String = Digest.sha256Hash(bytes).contentHashStr
override def input: InputStream = new java.io.ByteArrayInputStream(bytes) override def input: InputStream = new java.io.ByteArrayInputStream(bytes)
override def toPath(): Path = Paths.get(path)
end ActionCacheTest end ActionCacheTest