Avoid unnecessary insertion of tree file artifacts to artifactData.
Tree file artifacts that are not template expansion outputs are already stored as part of their parent artifact in treeArtifactData, so they do not need to also be stored in artifactData. This is accomplished by having callers insert metadata into the store instead of doing it in constructFileArtifactValue and maybeStoreAdditionalData.
Additionally, constructFileArtifactValue and maybeStoreAdditionalData are merged into a single method since they are always called in succession, and the latter's name doesn't make much sense after https://github.com/bazelbuild/bazel/commit/c35878a258d3c8a5305389aadd17ac0049defa15 removed additionalOutputData. Outdated comments are cleaned up.
FilesystemValueChecker made some assumptions about tree files appearing in both maps except in the remote case, so had to change the logic there a bit.
RELNOTES: None.
PiperOrigin-RevId: 313452251
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
index dd58b95..ad9354b 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
@@ -871,19 +871,16 @@
}
return ActionExecutionValue.create(
artifactData,
- ImmutableMap.<Artifact, TreeArtifactValue>of(),
+ /*treeArtifactData=*/ ImmutableMap.of(),
/*outputSymlinks=*/ null,
/*discoveredModules=*/ null,
/*actionDependsOnBuildId=*/ false);
}
- private static ActionExecutionValue actionValueWithEmptyDirectory(Artifact emptyDir) {
- TreeArtifactValue emptyValue = TreeArtifactValue.create
- (ImmutableMap.<TreeFileArtifact, FileArtifactValue>of());
-
+ private static ActionExecutionValue actionValueWithEmptyDirectory(SpecialArtifact emptyDir) {
return ActionExecutionValue.create(
- ImmutableMap.of(),
- ImmutableMap.of(emptyDir, emptyValue),
+ /*artifactData=*/ ImmutableMap.of(),
+ ImmutableMap.of(emptyDir, TreeArtifactValue.create(ImmutableMap.of())),
/*outputSymlinks=*/ null,
/*discoveredModules=*/ null,
/*actionDependsOnBuildId=*/ false);
@@ -891,7 +888,6 @@
private static ActionExecutionValue actionValueWithTreeArtifacts(
List<TreeFileArtifact> contents) {
- Map<Artifact, FileArtifactValue> fileData = new HashMap<>();
Map<Artifact, Map<TreeFileArtifact, FileArtifactValue>> directoryData = new HashMap<>();
for (TreeFileArtifact output : contents) {
@@ -912,7 +908,6 @@
FileArtifactValue.createFromInjectedDigest(
noDigest, path.getDigest(), !output.isConstantMetadata());
dirDatum.put(output, withDigest);
- fileData.put(output, withDigest);
} catch (IOException e) {
throw new IllegalStateException(e);
}
@@ -925,7 +920,7 @@
}
return ActionExecutionValue.create(
- fileData,
+ /*artifactData=*/ ImmutableMap.of(),
treeArtifactData,
/*outputSymlinks=*/ null,
/*discoveredModules=*/ null,