Split FileAndMetadataCache into two classes, since most of the shared functionality is gone.
--
MOS_MIGRATED_REVID=90289916
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
index 9861aa1..cf15343 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
@@ -278,29 +278,28 @@
return skyframeActionExecutor.executeAction(action, null, -1, null);
}
// This may be recreated if we discover inputs.
- FileAndMetadataCache fileAndMetadataCache = new FileAndMetadataCache(
- state.inputArtifactData,
- state.expandedMiddlemen,
- skyframeActionExecutor.getExecRoot(),
- action.getOutputs(),
- tsgm);
+ ActionMetadataHandler metadataHandler = new ActionMetadataHandler(state.inputArtifactData,
+ action.getOutputs(), tsgm);
long actionStartTime = System.nanoTime();
// We only need to check the action cache if we haven't done it on a previous run.
if (!state.hasCheckedActionCache()) {
- state.token = skyframeActionExecutor.checkActionCache(action, fileAndMetadataCache,
+ state.token = skyframeActionExecutor.checkActionCache(action, metadataHandler,
actionStartTime, state.allInputs.actionCacheInputs);
}
if (state.token == null) {
// We got a hit from the action cache -- no need to execute.
return new ActionExecutionValue(
- fileAndMetadataCache.getOutputData(),
- fileAndMetadataCache.getAdditionalOutputData());
+ metadataHandler.getOutputData(),
+ metadataHandler.getAdditionalOutputData());
}
// This may be recreated if we discover inputs.
+ PerActionFileCache perActionFileCache =
+ new PerActionFileCache(state.inputArtifactData, skyframeActionExecutor.getExecRoot());
ActionExecutionContext actionExecutionContext =
- skyframeActionExecutor.constructActionExecutionContext(fileAndMetadataCache);
+ skyframeActionExecutor.constructActionExecutionContext(perActionFileCache,
+ metadataHandler, state.expandedMiddlemen);
boolean inputsDiscoveredDuringActionExecution = false;
Map<Artifact, FileArtifactValue> metadataFoundDuringActionExecution = null;
try {
@@ -324,20 +323,18 @@
return null;
}
state.inputArtifactData = inputArtifactData;
- fileAndMetadataCache = new FileAndMetadataCache(
- state.inputArtifactData,
- state.expandedMiddlemen,
- skyframeActionExecutor.getExecRoot(),
- action.getOutputs(),
- tsgm
- );
+ perActionFileCache =
+ new PerActionFileCache(state.inputArtifactData, skyframeActionExecutor.getExecRoot());
+ metadataHandler =
+ new ActionMetadataHandler(state.inputArtifactData, action.getOutputs(), tsgm);
actionExecutionContext =
- skyframeActionExecutor.constructActionExecutionContext(fileAndMetadataCache);
+ skyframeActionExecutor.constructActionExecutionContext(perActionFileCache,
+ metadataHandler, state.expandedMiddlemen);
}
}
if (!state.hasExecutedAction()) {
state.value = skyframeActionExecutor.executeAction(action,
- fileAndMetadataCache, actionStartTime, actionExecutionContext);
+ metadataHandler, actionStartTime, actionExecutionContext);
}
} finally {
try {
@@ -362,15 +359,10 @@
inputArtifactData.putAll(state.inputArtifactData);
inputArtifactData.putAll(metadataFoundDuringActionExecution);
state.inputArtifactData = inputArtifactData;
- fileAndMetadataCache = new FileAndMetadataCache(
- state.inputArtifactData,
- state.expandedMiddlemen,
- skyframeActionExecutor.getExecRoot(),
- action.getOutputs(),
- tsgm
- );
+ metadataHandler =
+ new ActionMetadataHandler(state.inputArtifactData, action.getOutputs(), tsgm);
}
- skyframeActionExecutor.afterExecution(action, fileAndMetadataCache, state.token);
+ skyframeActionExecutor.afterExecution(action, metadataHandler, state.token);
return state.value;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java
index de63c3b..714325a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java
@@ -42,7 +42,7 @@
* @param artifactData Map from Artifacts to corresponding FileValues.
* @param additionalOutputData Map from Artifacts to values if the FileArtifactValue for this
* artifact cannot be derived from the corresponding FileValue (see {@link
- * FileAndMetadataCache#getAdditionalOutputData} for when this is necessary).
+ * ActionMetadataHandler#getAdditionalOutputData} for when this is necessary).
*/
ActionExecutionValue(Map<Artifact, FileValue> artifactData,
Map<Artifact, FileArtifactValue> additionalOutputData) {
@@ -53,7 +53,7 @@
/**
* Returns metadata for a given artifact, if that metadata cannot be inferred from the
* corresponding {@link #getData} call for that Artifact. See {@link
- * FileAndMetadataCache#getAdditionalOutputData} for when that can happen.
+ * ActionMetadataHandler#getAdditionalOutputData} for when that can happen.
*/
@Nullable
FileArtifactValue getArtifactValue(Artifact artifact) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileAndMetadataCache.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
similarity index 86%
rename from src/main/java/com/google/devtools/build/lib/skyframe/FileAndMetadataCache.java
rename to src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
index b9b1284..3230af8 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FileAndMetadataCache.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
@@ -17,12 +17,8 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Interner;
-import com.google.common.collect.Interners;
import com.google.common.collect.Sets;
-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;
import com.google.devtools.build.lib.actions.cache.Digest;
import com.google.devtools.build.lib.actions.cache.DigestUtils;
@@ -35,12 +31,9 @@
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.lib.vfs.Symlinks;
-import com.google.protobuf.ByteString;
-import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
-import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Collection;
import java.util.Map;
@@ -67,12 +60,9 @@
* versions of the data for a value (see {@link #getAdditionalOutputData} for more.
*/
@VisibleForTesting
-public class FileAndMetadataCache implements ActionInputFileCache, MetadataHandler {
+public class ActionMetadataHandler implements MetadataHandler {
/** This should never be read directly. Use {@link #getInputFileArtifactValue} instead. */
private final Map<Artifact, FileArtifactValue> inputArtifactData;
- private final Map<Artifact, Collection<Artifact>> expandedInputMiddlemen;
- private final File execRoot;
- private final Map<ByteString, Artifact> reverseMap = new ConcurrentHashMap<>();
private final ConcurrentMap<Artifact, FileValue> outputArtifactData =
new ConcurrentHashMap<>();
private final Set<Artifact> omittedOutputs = Sets.newConcurrentHashSet();
@@ -83,16 +73,11 @@
private final ImmutableSet<Artifact> outputs;
private final TimestampGranularityMonitor tsgm;
- private static final Interner<ByteString> BYTE_INTERNER = Interners.newWeakInterner();
-
@VisibleForTesting
- public FileAndMetadataCache(Map<Artifact, FileArtifactValue> inputArtifactData,
- Map<Artifact, Collection<Artifact>> expandedInputMiddlemen, File execRoot,
+ public ActionMetadataHandler(Map<Artifact, FileArtifactValue> inputArtifactData,
Iterable<Artifact> outputs,
TimestampGranularityMonitor tsgm) {
this.inputArtifactData = Preconditions.checkNotNull(inputArtifactData);
- this.expandedInputMiddlemen = Preconditions.checkNotNull(expandedInputMiddlemen);
- this.execRoot = Preconditions.checkNotNull(execRoot);
this.outputs = ImmutableSet.copyOf(outputs);
this.tsgm = tsgm;
}
@@ -188,14 +173,6 @@
return maybeStoreAdditionalData(artifact, fileValue, null);
}
- /** Expands one of the input middlemen artifacts of the corresponding action. */
- public Collection<Artifact> expandInputMiddleman(Artifact middlemanArtifact) {
- Preconditions.checkState(middlemanArtifact.isMiddlemanArtifact(), middlemanArtifact);
- Collection<Artifact> result = expandedInputMiddlemen.get(middlemanArtifact);
- // Note that result may be null for non-aggregating middlemen.
- return result == null ? ImmutableSet.<Artifact>of() : result;
- }
-
/**
* Check that the new {@code data} we just calculated for an {@code artifact} agrees with the
* {@code oldData} (presumably calculated concurrently), if it was present.
@@ -283,7 +260,7 @@
// should throw, since all filesystem access has already been done.
throw new IllegalStateException(
"Filesystem should not have been accessed while injecting data for "
- + artifact.prettyPrint(), e);
+ + artifact.prettyPrint(), e);
}
// Ignore exceptions for empty files, as above.
}
@@ -361,50 +338,9 @@
return additionalOutputData;
}
- @Override
- public long getSizeInBytes(ActionInput input) throws IOException {
- FileArtifactValue metadata = getInputFileArtifactValue(input);
- if (metadata != null) {
- return metadata.getSize();
- }
- return -1;
- }
-
- @Nullable
- @Override
- public File getFileFromDigest(ByteString digest) throws IOException {
- Artifact artifact = reverseMap.get(digest);
- if (artifact != null) {
- String relPath = artifact.getExecPathString();
- return relPath.startsWith("/") ? new File(relPath) : new File(execRoot, relPath);
- }
- return null;
- }
-
- @Nullable
- @Override
- public ByteString getDigest(ActionInput input) throws IOException {
- FileArtifactValue value = getInputFileArtifactValue(input);
- if (value != null) {
- byte[] bytes = value.getDigest();
- if (bytes != null) {
- ByteString digest = ByteString.copyFrom(BaseEncoding.base16().lowerCase().encode(bytes)
- .getBytes(StandardCharsets.US_ASCII));
- reverseMap.put(BYTE_INTERNER.intern(digest), (Artifact) input);
- return digest;
- }
- }
- return null;
- }
-
- @Override
- public boolean contentsAvailableLocally(ByteString digest) {
- return reverseMap.containsKey(digest);
- }
-
static FileValue fileValueFromArtifact(Artifact artifact,
@Nullable FileStatusWithDigest statNoFollow, TimestampGranularityMonitor tsgm)
- throws IOException {
+ throws IOException {
Path path = artifact.getPath();
RootedPath rootedPath =
RootedPath.toRootedPath(artifact.getRoot().getPath(), artifact.getRootRelativePath());
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java b/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
index d2e962a..d159214 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
@@ -214,7 +214,7 @@
SkyKey key = keyAndValue.getFirst();
FileValue lastKnownData = actionValue.getAllOutputArtifactData().get(artifact);
try {
- FileValue newData = FileAndMetadataCache.fileValueFromArtifact(artifact, stat, tsgm);
+ FileValue newData = ActionMetadataHandler.fileValueFromArtifact(artifact, stat, tsgm);
if (!newData.equals(lastKnownData)) {
updateIntraBuildModifiedCounter(stat != null ? stat.getLastChangeTime() : -1);
modifiedOutputFilesCounter.getAndIncrement();
@@ -272,7 +272,7 @@
Artifact artifact = entry.getKey();
FileValue lastKnownData = entry.getValue();
try {
- FileValue fileValue = FileAndMetadataCache.fileValueFromArtifact(artifact, null, tsgm);
+ FileValue fileValue = ActionMetadataHandler.fileValueFromArtifact(artifact, null, tsgm);
if (!fileValue.equals(lastKnownData)) {
updateIntraBuildModifiedCounter(fileValue.exists()
? fileValue.realRootedPath().asPath().getLastModifiedTime()
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
new file mode 100644
index 0000000..5dde5a5
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PerActionFileCache.java
@@ -0,0 +1,107 @@
+// Copyright 2014 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.skyframe;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Interner;
+import com.google.common.collect.Interners;
+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;
+import com.google.protobuf.ByteString;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import javax.annotation.Nullable;
+
+/**
+ * Cache provided by an {@link ActionExecutionFunction}, allowing Blaze to obtain artifact metadata
+ * from the graph.
+ *
+ * <p>Data for the action's inputs is injected into this cache on construction, using the graph as
+ * the source of truth.
+ */
+class PerActionFileCache implements ActionInputFileCache {
+ private final Map<Artifact, FileArtifactValue> inputArtifactData;
+ private final File execRoot;
+ // Populated lazily, on calls to #getDigest.
+ private final Map<ByteString, Artifact> reverseMap = new ConcurrentHashMap<>();
+
+ private static final Interner<ByteString> BYTE_INTERNER = Interners.newWeakInterner();
+
+ /**
+ * @param inputArtifactData Map from artifact to metadata, used to return metadata upon request.
+ * @param execRoot Path to the execution root, used to convert Artifacts' relative paths into
+ * absolute ones in the execution root.
+ */
+ PerActionFileCache(Map<Artifact, FileArtifactValue> inputArtifactData,
+ File execRoot) {
+ this.inputArtifactData = Preconditions.checkNotNull(inputArtifactData);
+ this.execRoot = Preconditions.checkNotNull(execRoot);
+ }
+
+ @Nullable
+ private FileArtifactValue getInputFileArtifactValue(ActionInput input) {
+ if (!(input instanceof Artifact)) {
+ return null;
+ }
+ return Preconditions.checkNotNull(inputArtifactData.get(input), input);
+ }
+
+ @Override
+ public long getSizeInBytes(ActionInput input) throws IOException {
+ FileArtifactValue metadata = getInputFileArtifactValue(input);
+ if (metadata != null) {
+ return metadata.getSize();
+ }
+ return -1;
+ }
+
+ @Nullable
+ @Override
+ public File getFileFromDigest(ByteString digest) throws IOException {
+ Artifact artifact = reverseMap.get(digest);
+ if (artifact != null) {
+ String relPath = artifact.getExecPathString();
+ return new File(execRoot, relPath);
+ }
+ return null;
+ }
+
+ @Nullable
+ @Override
+ public ByteString getDigest(ActionInput input) throws IOException {
+ FileArtifactValue value = getInputFileArtifactValue(input);
+ if (value != null) {
+ byte[] bytes = value.getDigest();
+ if (bytes != null) {
+ ByteString digest = ByteString.copyFrom(BaseEncoding.base16().lowerCase().encode(bytes)
+ .getBytes(StandardCharsets.US_ASCII));
+ reverseMap.put(BYTE_INTERNER.intern(digest), (Artifact) input);
+ return digest;
+ }
+ }
+ return null;
+ }
+
+ @Override
+ public boolean contentsAvailableLocally(ByteString digest) {
+ return reverseMap.containsKey(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 4dc1478..e76990c 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
@@ -367,7 +367,7 @@
*
* <p>For use from {@link ArtifactFunction} only.
*/
- ActionExecutionValue executeAction(Action action, FileAndMetadataCache graphFileCache,
+ ActionExecutionValue executeAction(Action action, ActionMetadataHandler metadataHandler,
long actionStartTime,
ActionExecutionContext actionExecutionContext)
throws ActionExecutionException, InterruptedException {
@@ -378,7 +378,7 @@
}
Artifact primaryOutput = action.getPrimaryOutput();
FutureTask<ActionExecutionValue> actionTask =
- new FutureTask<>(new ActionRunner(action, graphFileCache,
+ new FutureTask<>(new ActionRunner(action, metadataHandler,
actionStartTime, actionExecutionContext));
// Check to see if another action is already executing/has executed this value.
Pair<Action, FutureTask<ActionExecutionValue>> oldAction =
@@ -418,33 +418,40 @@
}
}
+ private static class MiddlemanExpanderImpl implements MiddlemanExpander {
+ private final Map<Artifact, Collection<Artifact>> expandedInputMiddlemen;
+
+ private MiddlemanExpanderImpl(Map<Artifact, Collection<Artifact>> expandedInputMiddlemen) {
+ this.expandedInputMiddlemen = expandedInputMiddlemen;
+ }
+
+ @Override
+ public void expand(Artifact middlemanArtifact, Collection<? super Artifact> output) {
+ Preconditions.checkState(middlemanArtifact.isMiddlemanArtifact(), middlemanArtifact);
+ Collection<Artifact> result = expandedInputMiddlemen.get(middlemanArtifact);
+ // Note that result may be null for non-aggregating middlemen.
+ if (result != null) {
+ output.addAll(result);
+ }
+ }
+ }
+
/**
* Returns an ActionExecutionContext suitable for executing a particular action. The caller should
* pass the returned context to {@link #executeAction}, and any other method that needs to execute
* tasks related to that action.
*/
ActionExecutionContext constructActionExecutionContext(
- final FileAndMetadataCache graphFileCache) {
+ PerActionFileCache graphFileCache, MetadataHandler metadataHandler,
+ Map<Artifact, Collection<Artifact>> expandedInputMiddlemen) {
// TODO(bazel-team): this should be closed explicitly somewhere.
FileOutErr fileOutErr = actionLogBufferPathGenerator.generate();
return new ActionExecutionContext(
executorEngine,
new DelegatingPairFileCache(graphFileCache, perBuildFileCache),
- graphFileCache,
+ metadataHandler,
fileOutErr,
- new MiddlemanExpander() {
- @Override
- public void expand(Artifact middlemanArtifact,
- Collection<? super Artifact> output) {
- // Legacy code is more permissive regarding "mm" in that it expands any middleman,
- // not just inputs of this action. Skyframe doesn't have access to a global action
- // graph, therefore this implementation can't expand any middleman, only the
- // inputs of this action.
- // This is fine though: actions should only hold references to their input
- // artifacts, otherwise hermeticity would be violated.
- output.addAll(graphFileCache.expandInputMiddleman(middlemanArtifact));
- }
- });
+ new MiddlemanExpanderImpl(expandedInputMiddlemen));
}
/**
@@ -542,15 +549,15 @@
private class ActionRunner implements Callable<ActionExecutionValue> {
private final Action action;
- private final FileAndMetadataCache graphFileCache;
+ private final ActionMetadataHandler metadataHandler;
private long actionStartTime;
private ActionExecutionContext actionExecutionContext;
- ActionRunner(Action action, FileAndMetadataCache graphFileCache,
+ ActionRunner(Action action, ActionMetadataHandler metadataHandler,
long actionStartTime,
ActionExecutionContext actionExecutionContext) {
this.action = action;
- this.graphFileCache = graphFileCache;
+ this.metadataHandler = metadataHandler;
this.actionStartTime = actionStartTime;
this.actionExecutionContext = actionExecutionContext;
}
@@ -580,7 +587,7 @@
prepareScheduleExecuteAndCompleteAction(action, actionExecutionContext, actionStartTime);
return new ActionExecutionValue(
- graphFileCache.getOutputData(), graphFileCache.getAdditionalOutputData());
+ metadataHandler.getOutputData(), metadataHandler.getAdditionalOutputData());
} finally {
profiler.completeTask(ProfilerTask.ACTION);
}
