Record the contents proxy after verifying a present file by digest (https://github.com/bazelbuild/bazel/pull/30871) ### Description When the prefetcher finds a file already present in the local filesystem and its metadata carries no (matching) contents proxy, it verifies the file by digest before skipping the download, but previously left the metadata unchanged, so every subsequent modification check had to digest the file again. Record the verified file's contents proxy just like after a fresh download so that future checks only require a stat. ### Motivation ### Build API Changes No ### Checklist - [x] I have added tests for the new use cases (if any). - [ ] I have updated the documentation (if applicable). ### Release Notes RELNOTES: None Closes #30871. PiperOrigin-RevId: 973847507 Change-Id: Iabce6e9399831895dcb3595bfc2d2d1b1ddb30ed
diff --git a/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java b/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java index 75ba0a3..b8665c3 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java +++ b/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java
@@ -279,7 +279,13 @@ if (digest == null) { digest = path.getDigest(); } - return !Arrays.equals(digest, metadata.getDigest()); + if (!Arrays.equals(digest, metadata.getDigest())) { + return true; + } + // The file contents have been verified to be up to date. Record the contents proxy when + // supported, just like after a fresh download, to make future modification checks cheaper. + metadata.setContentsProxy(FileContentsProxy.create(stat)); + return false; } protected abstract boolean canDownloadFile(Path path, FileArtifactValue metadata);
diff --git a/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java b/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java index 78b868a..1b8e6a0 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java +++ b/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java
@@ -53,6 +53,7 @@ import com.google.devtools.build.lib.actions.ArtifactRoot.RootType; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.FileArtifactValue; +import com.google.devtools.build.lib.actions.FileContentsProxy; import com.google.devtools.build.lib.actions.StaticInputMetadataProvider; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.remote.AbstractActionInputPrefetcher.MetadataSupplier; @@ -312,6 +313,26 @@ } @Test + public void prefetchFiles_fileExists_recordsContentsProxy() + throws IOException, ExecException, InterruptedException { + Map<ActionInput, FileArtifactValue> metadata = new HashMap<>(); + Map<HashCode, byte[]> cas = new HashMap<>(); + Artifact a = createRemoteArtifact("file", "hello world", metadata, cas); + FileSystemUtils.writeContent(a.getPath(), "hello world".getBytes(UTF_8)); + AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); + assertThat(metadata.get(a).getContentsProxy()).isNull(); + + wait( + prefetcher.prefetchFilesInterruptibly( + action, metadata.keySet(), metadata::get, Priority.MEDIUM, Reason.INPUTS)); + + // The already present file was verified to be up to date by digest and its contents proxy was + // recorded to make future modification checks cheaper. + assertThat(metadata.get(a).getContentsProxy()) + .isEqualTo(FileContentsProxy.create(a.getPath().stat())); + } + + @Test public void prefetchFiles_fileExistsButContentMismatches_download() throws IOException, ExecException, InterruptedException { Map<ActionInput, FileArtifactValue> metadata = new HashMap<>();