Make explicit that no I/O is done when constructing a FileArtifactValue for an ordinary Artifact in ArtifactFunction.
If the data available in the FileValue is not sufficient to recreate the FileArtifactValue without disk access, then ActionMetadataHandler already stores the FileArtifactValue separately.
This is approximately step -3 in a grand plan to save a bunch more memory when building with --batch --discard_analysis_cache --keep_going.
--
PiperOrigin-RevId: 150646007
MOS_MIGRATED_REVID=150646007
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
index fad6603..293f530 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
@@ -112,7 +112,7 @@
return createAggregatingValue(artifact, action,
actionValue.getArtifactValue(artifact), env);
} else {
- return createSimpleFileArtifactValue(artifact, action, actionValue, env);
+ return createSimpleFileArtifactValue(artifact, actionValue);
}
}
}
@@ -157,8 +157,8 @@
treeArtifact);
for (TreeFileArtifact treeFileArtifact : treeFileArtifacts) {
- FileArtifactValue value = createSimpleFileArtifactValue(
- treeFileArtifact, expandedAction, actionExecutionValue, env);
+ FileArtifactValue value =
+ createSimpleFileArtifactValue(treeFileArtifact, actionExecutionValue);
map.put(treeFileArtifact, value);
}
}
@@ -224,9 +224,8 @@
// Non-aggregating artifact -- should contain at most one piece of artifact data.
// data may be null if and only if artifact is a middleman artifact.
- private static FileArtifactValue createSimpleFileArtifactValue(Artifact artifact,
- Action generatingAction, ActionExecutionValue actionValue, Environment env)
- throws ArtifactFunctionException {
+ private static FileArtifactValue createSimpleFileArtifactValue(
+ Artifact artifact, ActionExecutionValue actionValue) throws ArtifactFunctionException {
FileArtifactValue value = actionValue.getArtifactValue(artifact);
if (value != null) {
return value;
@@ -238,16 +237,10 @@
"%s %s", artifact, actionValue);
Preconditions.checkNotNull(data.getDigest(),
"Digest should already have been calculated for %s (%s)", artifact, data);
-
- try {
- return FileArtifactValue.create(artifact, data);
- } catch (IOException e) {
- ActionExecutionException ex = new ActionExecutionException(e, generatingAction,
- /*catastrophe=*/false);
- env.getListener().handle(Event.error(ex.getLocation(), ex.getMessage()));
- // This is a transient error since we did the work that led to the IOException.
- throw new ArtifactFunctionException(ex, Transience.TRANSIENT);
- }
+ // Directories are special-cased because their mtimes are used, so should have been constructed
+ // during execution of the action (in ActionMetadataHandler#maybeStoreAdditionalData).
+ Preconditions.checkState(data.isFile(), "Unexpected not file %s (%s)", artifact, data);
+ return FileArtifactValue.createNormalFile(data.getDigest(), data.getSize());
}
private static AggregatingArtifactValue createAggregatingValue(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileArtifactValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileArtifactValue.java
index dcbe1bc..cfdcb187 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FileArtifactValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileArtifactValue.java
@@ -236,6 +236,10 @@
return new DirectoryArtifactValue(artifact.getPath().getLastModifiedTime());
}
Preconditions.checkState(digest != null, artifact);
+ return createNormalFile(digest, size);
+ }
+
+ static FileArtifactValue createNormalFile(byte[] digest, long size) {
return new RegularFileArtifactValue(digest, size);
}
@@ -245,7 +249,7 @@
*/
static FileArtifactValue createProxy(byte[] digest) {
Preconditions.checkNotNull(digest);
- return new RegularFileArtifactValue(digest, /*size=*/ 0);
+ return createNormalFile(digest, /*size=*/ 0);
}
/** Returns the digest of this value. Null for non-files, non-null for files. */
