Skip to content

Commit

Permalink
Rename MetadataProvider to InputMetadataProvider and MetadataHandler …
Browse files Browse the repository at this point in the history 
…to OutputMetadataStore.

This is to better underline that one deals with action inputs, the other, with action outputs.

This change was generated mostly mechanically using my IDE, with manual tweaks to fix up the annoying "/* parameterName= */" comments.

RELNOTES: None.
PiperOrigin-RevId: 523743519
Change-Id: Ia7a67ad4296e82c31d50f47045663f2a6f151299
  • Loading branch information
@lberki @copybara-github
lberki authored and copybara-github committed Apr 12, 2023
1 parent b2d1ca9 commit 8ec4c50
Show file tree
Hide file tree
Showing 57 changed files with 426 additions and 392 deletions.
12 changes: 6 additions & 6 deletions src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import com.google.common.flogger.GoogleLogger;
import com.google.devtools.build.lib.actions.Artifact.ArchivedTreeArtifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.cache.MetadataHandler;
import com.google.devtools.build.lib.actions.cache.OutputMetadataStore;
import com.google.devtools.build.lib.actions.extra.ExtraActionInfo;
import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
import com.google.devtools.build.lib.cmdline.Label;
Expand Down Expand Up @@ -548,14 +548,14 @@ public static void deleteOutput(Path path, @Nullable Root root) throws IOExcepti
* checking, this method must be called.
*/
protected void checkInputsForDirectories(
EventHandler eventHandler, MetadataProvider metadataProvider) throws ExecException {
EventHandler eventHandler, InputMetadataProvider inputMetadataProvider) throws ExecException {
// Report "directory dependency checking" warning only for non-generated directories (generated
// ones will be reported earlier).
for (Artifact input : getMandatoryInputs().toList()) {
// Assume that if the file did not exist, we would not have gotten here.
try {
if (input.isSourceArtifact()
&& metadataProvider.getInputMetadata(input).getType().isDirectory()) {
&& inputMetadataProvider.getInputMetadata(input).getType().isDirectory()) {
// TODO(ulfjack): What about dependency checking of special files?
eventHandler.handle(
Event.warn(
Expand All @@ -581,12 +581,12 @@ protected void checkOutputsForDirectories(ActionExecutionContext actionExecution
throws InterruptedException {
FileArtifactValue metadata;
for (Artifact output : getOutputs()) {
MetadataHandler metadataHandler = actionExecutionContext.getMetadataHandler();
if (metadataHandler.artifactOmitted(output)) {
OutputMetadataStore outputMetadataStore = actionExecutionContext.getOutputMetadataStore();
if (outputMetadataStore.artifactOmitted(output)) {
continue;
}
try {
metadata = metadataHandler.getOutputMetadata(output);
metadata = outputMetadataStore.getOutputMetadata(output);
} catch (IOException e) {
logger.atWarning().withCause(e).log("Error getting metadata for %s", output);
metadata = null;
Expand Down
92 changes: 47 additions & 45 deletions src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
import com.google.devtools.build.lib.actions.cache.ActionCache;
import com.google.devtools.build.lib.actions.cache.ActionCache.Entry.SerializableTreeArtifactValue;
import com.google.devtools.build.lib.actions.cache.MetadataDigestUtils;
import com.google.devtools.build.lib.actions.cache.MetadataHandler;
import com.google.devtools.build.lib.actions.cache.OutputMetadataStore;
import com.google.devtools.build.lib.actions.cache.Protos.ActionCacheStatistics.MissReason;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
Expand Down Expand Up @@ -180,7 +180,7 @@ private static FileArtifactValue getCachedMetadata(
* @param action action to be validated.
* @param actionInputs the inputs of the action. Normally just the result of action.getInputs(),
* but if this action doesn't yet know its inputs, we check the inputs from the cache.
* @param metadataHandler provider of metadata for the artifacts this action interacts with.
* @param outputMetadataStore provider of metadata for the artifacts this action interacts with.
* @param checkOutput true to validate output artifacts, Otherwise, just validate inputs.
* @param cachedOutputMetadata a set of cached metadata that should be used instead of loading
* from {@code metadataHandler}.
Expand All @@ -190,8 +190,8 @@ private static boolean validateArtifacts(
ActionCache.Entry entry,
Action action,
NestedSet<Artifact> actionInputs,
MetadataProvider metadataProvider,
MetadataHandler metadataHandler,
InputMetadataProvider inputMetadataProvider,
OutputMetadataStore outputMetadataStore,
boolean checkOutput,
@Nullable CachedOutputMetadata cachedOutputMetadata)
throws InterruptedException {
Expand All @@ -200,14 +200,15 @@ private static boolean validateArtifacts(
for (Artifact artifact : action.getOutputs()) {
FileArtifactValue metadata = getCachedMetadata(cachedOutputMetadata, artifact);
if (metadata == null) {
metadata = getOutputMetadataMaybe(metadataHandler, artifact);
metadata = getOutputMetadataMaybe(outputMetadataStore, artifact);
}

mdMap.put(artifact.getExecPathString(), metadata);
}
}
for (Artifact artifact : actionInputs.toList()) {
mdMap.put(artifact.getExecPathString(), getInputMetadataMaybe(metadataProvider, artifact));
mdMap.put(
artifact.getExecPathString(), getInputMetadataMaybe(inputMetadataProvider, artifact));
}
return !Arrays.equals(MetadataDigestUtils.fromMetadata(mdMap), entry.getFileDigest());
}
Expand Down Expand Up @@ -319,7 +320,7 @@ private CachedOutputMetadata(
}

private static CachedOutputMetadata loadCachedOutputMetadata(
Action action, ActionCache.Entry entry, MetadataHandler metadataHandler)
Action action, ActionCache.Entry entry, OutputMetadataStore outputMetadataStore)
throws InterruptedException {
Instant now = Instant.now();
ImmutableMap.Builder<Artifact, RemoteFileArtifactValue> remoteFileMetadata =
Expand Down Expand Up @@ -348,7 +349,7 @@ private static CachedOutputMetadata loadCachedOutputMetadata(

TreeArtifactValue localTreeMetadata;
try {
localTreeMetadata = metadataHandler.getTreeArtifactValue(parent);
localTreeMetadata = outputMetadataStore.getTreeArtifactValue(parent);
} catch (IOException e) {
// Ignore the cached metadata if we encountered an error when loading corresponding
// local one.
Expand Down Expand Up @@ -399,7 +400,7 @@ private static CachedOutputMetadata loadCachedOutputMetadata(

FileArtifactValue localMetadata;
try {
localMetadata = getOutputMetadataOrConstant(metadataHandler, artifact);
localMetadata = getOutputMetadataOrConstant(outputMetadataStore, artifact);
} catch (FileNotFoundException ignored) {
localMetadata = null;
} catch (IOException e) {
Expand Down Expand Up @@ -439,8 +440,8 @@ public Token getTokenIfNeedToExecute(
Map<String, String> clientEnv,
OutputPermissions outputPermissions,
EventHandler handler,
MetadataProvider metadataProvider,
MetadataHandler metadataHandler,
InputMetadataProvider inputMetadataProvider,
OutputMetadataStore outputMetadataStore,
ArtifactExpander artifactExpander,
Map<String, String> remoteDefaultPlatformProperties,
boolean loadCachedOutputMetadata)
Expand All @@ -456,7 +457,7 @@ public Token getTokenIfNeedToExecute(
// Some types of middlemen are not checked because they should not
// propagate invalidation of their inputs.
if (middlemanType != MiddlemanType.SCHEDULING_DEPENDENCY_MIDDLEMAN) {
checkMiddlemanAction(action, handler, metadataProvider, metadataHandler);
checkMiddlemanAction(action, handler, inputMetadataProvider, outputMetadataStore);
}
return null;
}
Expand Down Expand Up @@ -490,15 +491,15 @@ public Token getTokenIfNeedToExecute(
&& cacheConfig.storeOutputMetadata()
&& loadCachedOutputMetadata) {
// load remote metadata from action cache
cachedOutputMetadata = loadCachedOutputMetadata(action, entry, metadataHandler);
cachedOutputMetadata = loadCachedOutputMetadata(action, entry, outputMetadataStore);
}

if (mustExecute(
action,
entry,
handler,
metadataProvider,
metadataHandler,
inputMetadataProvider,
outputMetadataStore,
artifactExpander,
actionInputs,
clientEnv,
Expand All @@ -517,8 +518,8 @@ public Token getTokenIfNeedToExecute(

// Inject cached output metadata if we have an action cache hit
if (cachedOutputMetadata != null) {
cachedOutputMetadata.remoteFileMetadata.forEach(metadataHandler::injectFile);
cachedOutputMetadata.mergedTreeMetadata.forEach(metadataHandler::injectTree);
cachedOutputMetadata.remoteFileMetadata.forEach(outputMetadataStore::injectFile);
cachedOutputMetadata.mergedTreeMetadata.forEach(outputMetadataStore::injectTree);
}

return null;
Expand All @@ -528,8 +529,8 @@ private boolean mustExecute(
Action action,
@Nullable ActionCache.Entry entry,
EventHandler handler,
MetadataProvider metadataProvider,
MetadataHandler metadataHandler,
InputMetadataProvider inputMetadataProvider,
OutputMetadataStore outputMetadataStore,
ArtifactExpander artifactExpander,
NestedSet<Artifact> actionInputs,
Map<String, String> clientEnv,
Expand Down Expand Up @@ -558,8 +559,8 @@ private boolean mustExecute(
entry,
action,
actionInputs,
metadataProvider,
metadataHandler,
inputMetadataProvider,
outputMetadataStore,
true,
cachedOutputMetadata)) {
reportChanged(handler, action);
Expand All @@ -584,16 +585,17 @@ private boolean mustExecute(
}

private static FileArtifactValue getInputMetadataOrConstant(
MetadataProvider metadataProvider, Artifact artifact) throws IOException {
FileArtifactValue metadata = metadataProvider.getInputMetadata(artifact);
InputMetadataProvider inputMetadataProvider, Artifact artifact) throws IOException {
FileArtifactValue metadata = inputMetadataProvider.getInputMetadata(artifact);
return (metadata != null && artifact.isConstantMetadata())
? ConstantMetadataValue.INSTANCE
: metadata;
}

private static FileArtifactValue getOutputMetadataOrConstant(
MetadataHandler metadataHandler, Artifact artifact) throws IOException, InterruptedException {
FileArtifactValue metadata = metadataHandler.getOutputMetadata(artifact);
OutputMetadataStore outputMetadataStore, Artifact artifact)
throws IOException, InterruptedException {
FileArtifactValue metadata = outputMetadataStore.getOutputMetadata(artifact);
return (metadata != null && artifact.isConstantMetadata())
? ConstantMetadataValue.INSTANCE
: metadata;
Expand All @@ -604,9 +606,9 @@ private static FileArtifactValue getOutputMetadataOrConstant(
// should propagate the exception, because it is unexpected (e.g., bad file system state).
@Nullable
private static FileArtifactValue getInputMetadataMaybe(
MetadataProvider metadataProvider, Artifact artifact) {
InputMetadataProvider inputMetadataProvider, Artifact artifact) {
try {
return getInputMetadataOrConstant(metadataProvider, artifact);
return getInputMetadataOrConstant(inputMetadataProvider, artifact);
} catch (IOException e) {
return null;
}
Expand All @@ -617,9 +619,9 @@ private static FileArtifactValue getInputMetadataMaybe(
// should propagate the exception, because it is unexpected (e.g., bad file system state).
@Nullable
private static FileArtifactValue getOutputMetadataMaybe(
MetadataHandler metadataHandler, Artifact artifact) throws InterruptedException {
OutputMetadataStore outputMetadataStore, Artifact artifact) throws InterruptedException {
try {
return getOutputMetadataOrConstant(metadataHandler, artifact);
return getOutputMetadataOrConstant(outputMetadataStore, artifact);
} catch (IOException e) {
return null;
}
Expand All @@ -628,8 +630,8 @@ private static FileArtifactValue getOutputMetadataMaybe(
public void updateActionCache(
Action action,
Token token,
MetadataProvider metadataProvider,
MetadataHandler metadataHandler,
InputMetadataProvider inputMetadataProvider,
OutputMetadataStore outputMetadataStore,
ArtifactExpander artifactExpander,
Map<String, String> clientEnv,
OutputPermissions outputPermissions,
Expand All @@ -656,17 +658,17 @@ public void updateActionCache(
if (!key.equals(execPath)) {
actionCache.remove(execPath);
}
if (!metadataHandler.artifactOmitted(output)) {
if (!outputMetadataStore.artifactOmitted(output)) {
if (output.isTreeArtifact()) {
SpecialArtifact parent = (SpecialArtifact) output;
TreeArtifactValue metadata = metadataHandler.getTreeArtifactValue(parent);
TreeArtifactValue metadata = outputMetadataStore.getTreeArtifactValue(parent);
entry.addOutputTree(parent, metadata, cacheConfig.storeOutputMetadata());
} else {
// Output files *must* exist and be accessible after successful action execution. We use
// the 'constant' metadata for the volatile workspace status output. The volatile output
// contains information such as timestamps, and even when --stamp is enabled, we don't
// want to rebuild everything if only that file changes.
FileArtifactValue metadata = getOutputMetadataOrConstant(metadataHandler, output);
FileArtifactValue metadata = getOutputMetadataOrConstant(outputMetadataStore, output);
checkState(metadata != null);
entry.addOutputFile(output, metadata, cacheConfig.storeOutputMetadata());
}
Expand All @@ -684,7 +686,7 @@ public void updateActionCache(
for (Artifact input : action.getInputs().toList()) {
entry.addInputFile(
input.getExecPath(),
getInputMetadataMaybe(metadataProvider, input),
getInputMetadataMaybe(inputMetadataProvider, input),
/* saveExecPath= */ !excludePathsFromActionCache.contains(input));
}
entry.getFileDigest();
Expand Down Expand Up @@ -768,8 +770,8 @@ public List<Artifact> getCachedInputs(Action action, PackageRootResolver resolve
private void checkMiddlemanAction(
Action action,
EventHandler handler,
MetadataProvider metadataProvider,
MetadataHandler metadataHandler)
InputMetadataProvider inputMetadataProvider,
OutputMetadataStore outputMetadataStore)
throws InterruptedException {
if (!cacheConfig.enabled()) {
// Action cache is disabled, don't generate digests.
Expand All @@ -788,8 +790,8 @@ private void checkMiddlemanAction(
entry,
action,
action.getInputs(),
metadataProvider,
metadataHandler,
inputMetadataProvider,
outputMetadataStore,
false,
/* cachedOutputMetadata= */ null)) {
reportChanged(handler, action);
Expand All @@ -809,12 +811,12 @@ private void checkMiddlemanAction(
for (Artifact input : action.getInputs().toList()) {
entry.addInputFile(
input.getExecPath(),
getInputMetadataMaybe(metadataProvider, input),
getInputMetadataMaybe(inputMetadataProvider, input),
/* saveExecPath= */ true);
}
}

metadataHandler.setDigestForVirtualArtifact(middleman, entry.getFileDigest());
outputMetadataStore.setDigestForVirtualArtifact(middleman, entry.getFileDigest());
if (changed) {
actionCache.put(cacheKey, entry);
} else {
Expand All @@ -831,8 +833,8 @@ public Token getTokenUnconditionallyAfterFailureToRecordActionCacheHit(
Map<String, String> clientEnv,
OutputPermissions outputPermissions,
EventHandler handler,
MetadataProvider metadataProvider,
MetadataHandler metadataHandler,
InputMetadataProvider inputMetadataProvider,
OutputMetadataStore outputMetadataStore,
ArtifactExpander artifactExpander,
Map<String, String> remoteDefaultPlatformProperties,
boolean loadCachedOutputMetadata)
Expand All @@ -846,8 +848,8 @@ public Token getTokenUnconditionallyAfterFailureToRecordActionCacheHit(
clientEnv,
outputPermissions,
handler,
metadataProvider,
metadataHandler,
inputMetadataProvider,
outputMetadataStore,
artifactExpander,
remoteDefaultPlatformProperties,
loadCachedOutputMetadata);
Expand Down
Loading

0 comments on commit 8ec4c50

Please sign in to comment.