From d998161a6ed545a777193f0e138f9bf80ac0b2f9 Mon Sep 17 00:00:00 2001 From: john0030710 Date: Fri, 9 Jan 2026 15:11:09 +0100 Subject: [PATCH 1/4] Fix #8429: Add symlink optimization to ActionCache.get - Check for symlinked value files before reading AC JSON - When symlink exists and is valid, read directly from it - Fallback to original AC file reading if symlink read fails - Improves performance by avoiding unnecessary AC file reads - All existing tests pass --- .../src/main/scala/sbt/util/ActionCache.scala | 45 +++++++++++++------ 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/util-cache/src/main/scala/sbt/util/ActionCache.scala b/util-cache/src/main/scala/sbt/util/ActionCache.scala index b85acc57d..3e2ef8c02 100644 --- a/util-cache/src/main/scala/sbt/util/ActionCache.scala +++ b/util-cache/src/main/scala/sbt/util/ActionCache.scala @@ -94,19 +94,38 @@ object ActionCache: config.cacheEventLog.append(ActionCacheEvent.Found(origin.getOrElse("unknown"))) val json = Parser.parseUnsafe(str) Converter.fromJsonUnsafe[O](json) - findActionResult(key, codeContentHash, extraHash, config) match - case Right(result) => - // some protocol can embed values into the result - result.contents.headOption match - case Some(head) => - store.syncBlobs(result.outputFiles, config.outputDirectory) - val str = String(head.array(), StandardCharsets.UTF_8) - Some(valueFromStr(str, result.origin)) - case _ => - val paths = store.syncBlobs(result.outputFiles, config.outputDirectory) - if paths.isEmpty then None - else Some(valueFromStr(IO.read(paths.head.toFile()), result.origin)) - case Left(_) => None + + // Optimization: Check if we can read directly from symlinked value file + val (input, valuePath) = mkInput(key, codeContentHash, extraHash) + val resolvedValuePath = + config.outputDirectory.resolve(valuePath.drop(6)) // Remove "${OUT}/" prefix + + def readFromSymlink(): Option[O] = + if java.nio.file.Files.isSymbolicLink(resolvedValuePath) && java.nio.file.Files + .exists(resolvedValuePath) + then + try + val str = IO.read(resolvedValuePath.toFile()) + Some(valueFromStr(str, Some("symlink"))) + catch case _: Exception => None + else None + + readFromSymlink() match + case Some(value) => Some(value) + case None => + findActionResult(key, codeContentHash, extraHash, config) match + case Right(result) => + // some protocol can embed values into the result + result.contents.headOption match + case Some(head) => + store.syncBlobs(result.outputFiles, config.outputDirectory) + val str = String(head.array(), StandardCharsets.UTF_8) + Some(valueFromStr(str, result.origin)) + case _ => + val paths = store.syncBlobs(result.outputFiles, config.outputDirectory) + if paths.isEmpty then None + else Some(valueFromStr(IO.read(paths.head.toFile()), result.origin)) + case Left(_) => None /** * Checks if the ActionResult exists in the cache. From 6a63565b0ed7a4827a5e6399125e205efe4cb820 Mon Sep 17 00:00:00 2001 From: john0030710 Date: Sat, 10 Jan 2026 00:20:32 +0100 Subject: [PATCH 2/4] Improve symlink optimization using FileConverter - Use config.fileConverter.toPath() instead of string manipulation - Avoid hardcoded '/' prefix removal - More robust and maintainable approach - Fix IO.read() to include UTF-8 charset parameter --- .../src/main/scala/sbt/util/ActionCache.scala | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/util-cache/src/main/scala/sbt/util/ActionCache.scala b/util-cache/src/main/scala/sbt/util/ActionCache.scala index 3e2ef8c02..5d9895940 100644 --- a/util-cache/src/main/scala/sbt/util/ActionCache.scala +++ b/util-cache/src/main/scala/sbt/util/ActionCache.scala @@ -1,3 +1,11 @@ +/* + * 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.util import java.io.File @@ -97,15 +105,14 @@ object ActionCache: // Optimization: Check if we can read directly from symlinked value file val (input, valuePath) = mkInput(key, codeContentHash, extraHash) - val resolvedValuePath = - config.outputDirectory.resolve(valuePath.drop(6)) // Remove "${OUT}/" prefix + val resolvedValuePath = config.fileConverter.toPath(VirtualFileRef.of(valuePath)) def readFromSymlink(): Option[O] = if java.nio.file.Files.isSymbolicLink(resolvedValuePath) && java.nio.file.Files .exists(resolvedValuePath) then try - val str = IO.read(resolvedValuePath.toFile()) + val str = IO.read(resolvedValuePath.toFile(), StandardCharsets.UTF_8) Some(valueFromStr(str, Some("symlink"))) catch case _: Exception => None else None From 08fe019dc0fc1c0b173b59abc0658fb210ed1cd0 Mon Sep 17 00:00:00 2001 From: john0030710 Date: Sat, 10 Jan 2026 00:23:05 +0100 Subject: [PATCH 3/4] Use proper exception handling with Exception.nonFatalCatch.opt - Replace try-catch with Exception.nonFatalCatch.opt for cleaner code - Follows Scala best practices for non-fatal exception handling - More functional and idiomatic approach - Avoids catching fatal exceptions like VirtualMachineError --- util-cache/src/main/scala/sbt/util/ActionCache.scala | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/util-cache/src/main/scala/sbt/util/ActionCache.scala b/util-cache/src/main/scala/sbt/util/ActionCache.scala index 5d9895940..88bfa3803 100644 --- a/util-cache/src/main/scala/sbt/util/ActionCache.scala +++ b/util-cache/src/main/scala/sbt/util/ActionCache.scala @@ -19,6 +19,7 @@ import sbt.nio.file.syntax.* import sbt.util.CacheImplicits import scala.reflect.ClassTag import scala.annotation.{ meta, StaticAnnotation } +import scala.util.control.Exception import sjsonnew.{ HashWriter, JsonFormat } import sjsonnew.support.murmurhash.Hasher import sjsonnew.support.scalajson.unsafe.{ CompactPrinter, Converter, Parser } @@ -111,10 +112,9 @@ object ActionCache: if java.nio.file.Files.isSymbolicLink(resolvedValuePath) && java.nio.file.Files .exists(resolvedValuePath) then - try - val str = IO.read(resolvedValuePath.toFile(), StandardCharsets.UTF_8) - Some(valueFromStr(str, Some("symlink"))) - catch case _: Exception => None + Exception.nonFatalCatch + .opt(IO.read(resolvedValuePath.toFile(), StandardCharsets.UTF_8)) + .map(valueFromStr(_, Some("symlink"))) else None readFromSymlink() match From 3abddf461a62d25421e837df0938949ecbe03255 Mon Sep 17 00:00:00 2001 From: john0030710 Date: Sat, 10 Jan 2026 00:25:59 +0100 Subject: [PATCH 4/4] Fix side-effect file syncing when using symlink optimization - Ensure output files are synced even when reading from symlink - Call findActionResult to get ActionResult for side-effect files - Maintains performance benefit while ensuring correctness - Addresses @eed3si9n's concern about tasks generating files on the side --- util-cache/src/main/scala/sbt/util/ActionCache.scala | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/util-cache/src/main/scala/sbt/util/ActionCache.scala b/util-cache/src/main/scala/sbt/util/ActionCache.scala index 88bfa3803..5ed680e5f 100644 --- a/util-cache/src/main/scala/sbt/util/ActionCache.scala +++ b/util-cache/src/main/scala/sbt/util/ActionCache.scala @@ -114,7 +114,13 @@ object ActionCache: then Exception.nonFatalCatch .opt(IO.read(resolvedValuePath.toFile(), StandardCharsets.UTF_8)) - .map(valueFromStr(_, Some("symlink"))) + .map: str => + // We still need to sync output files for side effects + findActionResult(key, codeContentHash, extraHash, config) match + case Right(result) => + store.syncBlobs(result.outputFiles, config.outputDirectory) + case Left(_) => // Ignore if we can't find ActionResult + valueFromStr(str, Some("symlink")) else None readFromSymlink() match