Removes most ActionInputFileCache functionality. Actual class to be removed in a later change. PiperOrigin-RevId: 198937695
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionInputFileCache.java b/src/main/java/com/google/devtools/build/lib/actions/ActionInputFileCache.java index 27b219d..a166f4e 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionInputFileCache.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionInputFileCache.java
@@ -13,42 +13,5 @@ // limitations under the License. package com.google.devtools.build.lib.actions; -import com.google.devtools.build.lib.vfs.Path; -import com.google.protobuf.ByteString; -import javax.annotation.Nullable; -import javax.annotation.concurrent.ThreadSafe; - -/** - * The interface for Action inputs metadata (Digest and size). - * - * NOTE: Implementations must be thread safe. - */ -@ThreadSafe -public interface ActionInputFileCache extends MetadataProvider { - /** - * Checks if the file is available locally, based on the assumption that previous operations on - * the ActionInputFileCache would have created a cache entry for it. - * - * @param digest the digest to lookup (as lowercase hex). - * @return true if the specified digest is backed by a locally-readable file, false otherwise - */ - boolean contentsAvailableLocally(ByteString digest); - - /** - * Concrete subclasses must implement this to provide a mapping from digest to file path, - * based on files previously seen as inputs. - * - * @param digest the digest (as lowercase hex). - * @return an ActionInput corresponding to the given digest. - */ - @Nullable - ActionInput getInputFromDigest(ByteString digest); - - /** - * The absolute path that this input is located at. The usual {@link ActionInput} implementation - * is {@link Artifact}, which currently embeds its full path, so implementations should just - * return this path if {@code input} is an {@link Artifact}. Otherwise, implementations should - * resolve the relative path into an absolute one and return that. - */ - Path getInputPath(ActionInput input); -} +/** A vestigial stub to be removed soon. */ +public interface ActionInputFileCache extends MetadataProvider {}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SingleBuildFileCache.java b/src/main/java/com/google/devtools/build/lib/exec/SingleBuildFileCache.java index 76479d0..7eace22 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SingleBuildFileCache.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SingleBuildFileCache.java
@@ -13,13 +13,10 @@ // limitations under the License. package com.google.devtools.build.lib.exec; -import static java.nio.charset.StandardCharsets.US_ASCII; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; -import com.google.common.collect.Maps; -import com.google.common.io.BaseEncoding; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputFileCache; import com.google.devtools.build.lib.actions.Artifact; @@ -28,17 +25,13 @@ import com.google.devtools.build.lib.skyframe.FileArtifactValue; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.Path; -import com.google.protobuf.ByteString; import java.io.IOException; -import java.util.Map; -import javax.annotation.Nullable; import javax.annotation.concurrent.ThreadSafe; /** * An in-memory cache to ensure we do I/O for source files only once during a single build. * - * <p>Simply maintains a two-way cached mapping from digest <--> filename that may be populated - * only once. + * <p>Simply maintains a cached mapping from filename to metadata that may be populated only once. */ @ThreadSafe public class SingleBuildFileCache implements ActionInputFileCache { @@ -73,12 +66,6 @@ throw new DigestOfDirectoryException( "Input is a directory: " + input.getExecPathString()); } - BaseEncoding hex = BaseEncoding.base16().lowerCase(); - ByteString hexDigest = - ByteString.copyFrom(hex.encode(metadata.getDigest()).getBytes(US_ASCII)); - // Inject reverse mapping. Doing this unconditionally in getDigest() showed up - // as a hotspot in CPU profiling. - digestToPath.put(hexDigest, input); return new ActionInputMetadata(metadata); } catch (IOException e) { return new ActionInputMetadata(e); @@ -86,29 +73,11 @@ } }); - private final Map<ByteString, ActionInput> digestToPath = Maps.newConcurrentMap(); - @Override public Metadata getMetadata(ActionInput input) throws IOException { return pathToMetadata.getUnchecked(input).getMetadata(); } - @Nullable - @Override - public ActionInput getInputFromDigest(ByteString digest) { - return digestToPath.get(digest); - } - - @Override - public Path getInputPath(ActionInput input) { - return execRoot.getRelative(input.getExecPath()); - } - - @Override - public boolean contentsAvailableLocally(ByteString digest) { - return digestToPath.containsKey(digest); - } - /** Container class for caching I/O around ActionInputs. */ private static class ActionInputMetadata { private final Metadata metadata;
diff --git a/src/main/java/com/google/devtools/build/lib/remote/TreeNodeRepository.java b/src/main/java/com/google/devtools/build/lib/remote/TreeNodeRepository.java index 8cbc619..6ffbd13 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/TreeNodeRepository.java +++ b/src/main/java/com/google/devtools/build/lib/remote/TreeNodeRepository.java
@@ -14,6 +14,8 @@ package com.google.devtools.build.lib.remote; +import static java.nio.charset.StandardCharsets.US_ASCII; + import com.google.common.base.Preconditions; import com.google.common.base.Predicate; import com.google.common.collect.ImmutableCollection; @@ -22,10 +24,11 @@ import com.google.common.collect.Interner; import com.google.common.collect.Iterables; import com.google.common.graph.Traverser; +import com.google.common.io.BaseEncoding; import com.google.devtools.build.lib.actions.ActionInput; -import com.google.devtools.build.lib.actions.ActionInputFileCache; import com.google.devtools.build.lib.actions.ActionInputHelper; import com.google.devtools.build.lib.actions.DigestOfDirectoryException; +import com.google.devtools.build.lib.actions.MetadataProvider; import com.google.devtools.build.lib.actions.cache.Metadata; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.concurrent.BlazeInterners; @@ -49,6 +52,7 @@ import java.util.Map; import java.util.Objects; import java.util.SortedMap; +import java.util.concurrent.ConcurrentHashMap; import javax.annotation.Nullable; /** @@ -57,6 +61,8 @@ */ @ThreadSafe public final class TreeNodeRepository { + private static final BaseEncoding LOWER_CASE_HEX = BaseEncoding.base16().lowerCase(); + // In this implementation, symlinks are NOT followed when expanding directory artifacts public static final Symlinks SYMLINK_POLICY = Symlinks.NOFOLLOW; @@ -217,7 +223,8 @@ // Merkle hashes are computed and cached by the repository, therefore execRoot must // be part of the state. private final Path execRoot; - private final ActionInputFileCache inputFileCache; + private final MetadataProvider inputFileCache; + private final Map<ByteString, ActionInput> reverseInputMap = new ConcurrentHashMap<>(); // For directories that are themselves artifacts, map of the ActionInput to the Merkle hash private final Map<ActionInput, Digest> inputDirectoryDigestCache = new HashMap<>(); private final Map<TreeNode, Digest> treeNodeDigestCache = new HashMap<>(); @@ -227,14 +234,13 @@ private final Map<Digest, VirtualActionInput> digestVirtualInputCache = new HashMap<>(); private final DigestUtil digestUtil; - public TreeNodeRepository( - Path execRoot, ActionInputFileCache inputFileCache, DigestUtil digestUtil) { + public TreeNodeRepository(Path execRoot, MetadataProvider inputFileCache, DigestUtil digestUtil) { this.execRoot = execRoot; this.inputFileCache = inputFileCache; this.digestUtil = digestUtil; } - public ActionInputFileCache getInputFileCache() { + public MetadataProvider getInputFileCache() { return inputFileCache; } @@ -321,9 +327,7 @@ ActionInput input = inputs.get(inputsStart); try { if (!(input instanceof VirtualActionInput) - && Preconditions.checkNotNull(inputFileCache.getMetadata(input)) - .getType() - .isDirectory()) { + && getInputMetadata(input).getType().isDirectory()) { Path leafPath = execRoot.getRelative(input.getExecPathString()); return interner.intern(new TreeNode(buildInputDirectoryEntries(leafPath), input)); } @@ -437,7 +441,7 @@ if (input instanceof VirtualActionInput) { return Preconditions.checkNotNull(virtualInputDigestCache.get(input)); } - Metadata metadata = Preconditions.checkNotNull(inputFileCache.getMetadata(input)); + Metadata metadata = getInputMetadata(input); byte[] digest = metadata.getDigest(); if (digest == null) { // If the artifact does not have a digest, it is because it is a directory. @@ -478,7 +482,7 @@ nodes.add(Preconditions.checkNotNull(directoryCache.get(treeNode))); } else { // If not there, it must be an ActionInput. ByteString hexDigest = ByteString.copyFromUtf8(digest.getHash()); - ActionInput input = inputFileCache.getInputFromDigest(hexDigest); + ActionInput input = reverseInputMap.get(hexDigest); if (input == null) { // ... or a VirtualActionInput. input = digestVirtualInputCache.get(digest); @@ -487,4 +491,16 @@ } } } + + private Metadata getInputMetadata(ActionInput input) throws IOException { + Metadata metadata = + Preconditions.checkNotNull( + inputFileCache.getMetadata(input), "Missing metadata for: %s", input); + if (metadata.getDigest() != null) { + reverseInputMap.put( + ByteString.copyFrom(LOWER_CASE_HEX.encode(metadata.getDigest()).getBytes(US_ASCII)), + input); + } + return metadata; + } }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionFileSystem.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionFileSystem.java index d6d695c..383150a 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionFileSystem.java
@@ -156,26 +156,6 @@ return getMetadataChecked(actionInput.getExecPath()); } - @Override - public boolean contentsAvailableLocally(ByteString digest) { - return optionalInputsByDigest.containsKey(digest) || inputArtifactData.contains(digest); - } - - @Override - @Nullable - public ActionInput getInputFromDigest(ByteString digest) { - Artifact artifact = optionalInputsByDigest.get(digest); - return artifact != null ? artifact : inputArtifactData.get(digest); - } - - @Override - public Path getInputPath(ActionInput actionInput) { - if (actionInput instanceof Artifact) { - return getPath(((Artifact) actionInput).getPath().asFragment()); - } - return execRootPath.getRelative(actionInput.getExecPath()); - } - // -------------------- FileSystem implementation -------------------- @Override
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/InputArtifactData.java b/src/main/java/com/google/devtools/build/lib/skyframe/InputArtifactData.java index bb05604..e65c617 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/InputArtifactData.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/InputArtifactData.java
@@ -23,7 +23,7 @@ import java.util.HashMap; import javax.annotation.Nullable; -/** An bidirectional mapping between artifacts and metadata. */ +/** A mapping from artifacts to metadata. */ interface InputArtifactData { boolean contains(ActionInput input); @@ -31,11 +31,6 @@ @Nullable FileArtifactValue get(ActionInput input); - boolean contains(ByteString digest); - - @Nullable - Artifact get(ByteString digest); - @Nullable FileArtifactValue get(PathFragment fragment); @@ -47,11 +42,9 @@ */ final class MutableInputArtifactData implements InputArtifactData { private final HashMap<PathFragment, FileArtifactValue> inputs; - private final HashMap<ByteString, Artifact> reverseMap; public MutableInputArtifactData(int sizeHint) { this.inputs = new HashMap<>(sizeHint); - this.reverseMap = new HashMap<>(sizeHint); } @Override @@ -66,17 +59,6 @@ } @Override - public boolean contains(ByteString digest) { - return reverseMap.containsKey(digest); - } - - @Override - @Nullable - public Artifact get(ByteString digest) { - return reverseMap.get(digest); - } - - @Override @Nullable public FileArtifactValue get(PathFragment fragment) { return inputs.get(fragment); @@ -84,9 +66,6 @@ public void put(Artifact artifact, FileArtifactValue value) { inputs.put(artifact.getExecPath(), value); - if (value.getType().exists() && value.getDigest() != null) { - reverseMap.put(toByteString(value.getDigest()), artifact); - } } @Override
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PerActionFileCache.java b/src/main/java/com/google/devtools/build/lib/skyframe/PerActionFileCache.java index 50ef5c6..b455a8e 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PerActionFileCache.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PerActionFileCache.java
@@ -18,8 +18,6 @@ import com.google.devtools.build.lib.actions.ActionInputFileCache; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.cache.Metadata; -import com.google.devtools.build.lib.vfs.Path; -import com.google.protobuf.ByteString; import javax.annotation.Nullable; /** @@ -53,20 +51,4 @@ Preconditions.checkState(missingArtifactsAllowed || result != null, "null for %s", input); return result; } - - @Override - public Path getInputPath(ActionInput input) { - return ((Artifact) input).getPath(); - } - - @Override - public boolean contentsAvailableLocally(ByteString digest) { - return inputArtifactData.contains(digest); - } - - @Nullable - @Override - public Artifact getInputFromDigest(ByteString digest) { - return inputArtifactData.get(digest); - } }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java index 20fca21..4f10f68 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
@@ -84,7 +84,6 @@ import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.lib.vfs.Symlinks; import com.google.devtools.build.skyframe.SkyFunction.Environment; -import com.google.protobuf.ByteString; import java.io.FileNotFoundException; import java.io.IOException; import java.util.Collection; @@ -1274,27 +1273,5 @@ ? metadata : perBuildFileCache.getMetadata(input); } - - @Override - public boolean contentsAvailableLocally(ByteString digest) { - return perActionCache.contentsAvailableLocally(digest) - || perBuildFileCache.contentsAvailableLocally(digest); - } - - @Nullable - @Override - public ActionInput getInputFromDigest(ByteString digest) { - ActionInput file = perActionCache.getInputFromDigest(digest); - return file != null ? file : perBuildFileCache.getInputFromDigest(digest); - } - - @Override - public Path getInputPath(ActionInput input) { - if (input instanceof Artifact) { - return perActionCache.getInputPath(input); - } else { - return perBuildFileCache.getInputPath(input); - } - } } }
diff --git a/src/test/java/com/google/devtools/build/lib/exec/SingleBuildFileCacheTest.java b/src/test/java/com/google/devtools/build/lib/exec/SingleBuildFileCacheTest.java index 5e87a65..7a94c79 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/SingleBuildFileCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/SingleBuildFileCacheTest.java
@@ -116,14 +116,7 @@ byte[] digestBytes = underTest.getMetadata(empty).getDigest(); ByteString digest = ByteString.copyFromUtf8( BaseEncoding.base16().lowerCase().encode(digestBytes)); - assertThat(digest.toStringUtf8()).isEqualTo(EMPTY_MD5); - assertThat(underTest.getInputFromDigest(digest).getExecPathString()).isEqualTo("/empty"); - assert(underTest.contentsAvailableLocally(digest)); - - ByteString other = ByteString.copyFrom("f41d8cd98f00b204e9800998ecf8427e", "UTF-16"); - assert(!underTest.contentsAvailableLocally(other)); - assert(calls.containsKey("/empty")); } @Test
diff --git a/src/test/java/com/google/devtools/build/lib/exec/util/FakeActionInputFileCache.java b/src/test/java/com/google/devtools/build/lib/exec/util/FakeActionInputFileCache.java index ac5f9a4..9c215fc 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/util/FakeActionInputFileCache.java +++ b/src/test/java/com/google/devtools/build/lib/exec/util/FakeActionInputFileCache.java
@@ -17,12 +17,9 @@ import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputFileCache; import com.google.devtools.build.lib.actions.cache.Metadata; -import com.google.devtools.build.lib.vfs.Path; -import com.google.protobuf.ByteString; import java.io.IOException; import java.util.HashMap; import java.util.Map; -import javax.annotation.Nullable; /** A fake implementation of the {@link ActionInputFileCache} interface. */ public final class FakeActionInputFileCache implements ActionInputFileCache { @@ -39,20 +36,4 @@ public Metadata getMetadata(ActionInput input) throws IOException { return Preconditions.checkNotNull(inputs.get(input)); } - - @Override - public boolean contentsAvailableLocally(ByteString digest) { - throw new UnsupportedOperationException(); - } - - @Override - @Nullable - public ActionInput getInputFromDigest(ByteString hexDigest) { - throw new UnsupportedOperationException(); - } - - @Override - public Path getInputPath(ActionInput input) { - throw new UnsupportedOperationException(); - } -} \ No newline at end of file +}
diff --git a/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java b/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java index 0fdf722..26cc290 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java +++ b/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java
@@ -29,9 +29,7 @@ import com.google.devtools.build.lib.vfs.Symlinks; import com.google.devtools.remoteexecution.v1test.Digest; import com.google.devtools.remoteexecution.v1test.Tree; -import com.google.protobuf.ByteString; import java.io.IOException; -import javax.annotation.Nullable; /** A fake implementation of the {@link ActionInputFileCache} interface. */ final class FakeActionInputFileCache implements ActionInputFileCache { @@ -57,23 +55,6 @@ cas.put(input, digest); } - @Override - public boolean contentsAvailableLocally(ByteString digest) { - throw new UnsupportedOperationException(); - } - - @Override - @Nullable - public ActionInput getInputFromDigest(ByteString hexDigest) { - HashCode code = HashCode.fromString(hexDigest.toStringUtf8()); - return Preconditions.checkNotNull(cas.inverse().get(code.toString())); - } - - @Override - public Path getInputPath(ActionInput input) { - throw new UnsupportedOperationException(); - } - public Digest createScratchInput(ActionInput input, String content) throws IOException { Path inputFile = execRoot.getRelative(input.getExecPath()); FileSystemUtils.createDirectoryAndParents(inputFile.getParentDirectory()); @@ -90,4 +71,4 @@ setDigest(input, digest.getHash()); return digest; } -} \ No newline at end of file +}