Reduce GC in ActionExecutionFunction caused by removal of MiddlemanArtifacts.
With the targets in b/169092361, turning off middlemen resulted in a 39% increase in GC.time This CL cuts that down to 13%, and also slightly improved the GC time (-3.4%) when Middlemen is enabled.
With NSOS in ActionExecutionFunction, we maintain a map from SkyKey -> Artifacts whose Artifact::key is the SkyKey. There's no need to do that for a big portion of input Artifacts of actions, whose corresponding SkyKeys are just themselves (e.g. SourceArtifacts). This CL removes such Artifacts from the map.
PiperOrigin-RevId: 336882444
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 0f3c141..e14f459 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
@@ -1317,17 +1317,20 @@
ImmutableList<Artifact> allInputsList = allInputs.toList();
// Some keys have more than 1 corresponding Artifact (e.g. actions with 2 outputs).
- Multimap<SkyKey, Artifact> skyKeyToArtifactOrSet =
+ // For Artifacts whose Artifact::key isn't itself.
+ Multimap<SkyKey, Artifact> skyKeyToArtifactSet =
MultimapBuilder.hashKeys().hashSetValues().build();
- allInputsList.forEach(input -> skyKeyToArtifactOrSet.put(Artifact.key(input), input));
+ allInputsList.forEach(
+ input -> {
+ SkyKey key = Artifact.key(input);
+ if (key != input) {
+ skyKeyToArtifactSet.put(key, input);
+ }
+ });
ActionExecutionFunctionExceptionHandler actionExecutionFunctionExceptionHandler =
new ActionExecutionFunctionExceptionHandler(
- skyKeyToArtifactOrSet,
- inputDeps,
- action,
- mandatoryInputs,
- skyframeActionExecutor);
+ skyKeyToArtifactSet, inputDeps, action, mandatoryInputs, skyframeActionExecutor);
actionExecutionFunctionExceptionHandler.accumulateAndThrowExceptions();
@@ -1536,7 +1539,7 @@
/** Helper subclass for the error-handling logic for ActionExecutionFunction#accumulateInputs. */
private static final class ActionExecutionFunctionExceptionHandler {
- private final Multimap<SkyKey, Artifact> skyKeyToArtifactSet;
+ private final Multimap<SkyKey, Artifact> skyKeyToDerivedArtifactSet;
private final Map<
SkyKey,
ValueOrException3<
@@ -1550,7 +1553,7 @@
private ActionExecutionException firstActionExecutionException;
ActionExecutionFunctionExceptionHandler(
- Multimap<SkyKey, Artifact> skyKeyToArtifactSet,
+ Multimap<SkyKey, Artifact> skyKeyToDerivedArtifactSet,
Map<
SkyKey,
ValueOrException3<
@@ -1559,7 +1562,7 @@
Action action,
Set<Artifact> mandatoryInputs,
SkyframeActionExecutor skyframeActionExecutor) {
- this.skyKeyToArtifactSet = skyKeyToArtifactSet;
+ this.skyKeyToDerivedArtifactSet = skyKeyToDerivedArtifactSet;
this.inputDeps = inputDeps;
this.action = action;
this.mandatoryInputs = mandatoryInputs;
@@ -1584,13 +1587,9 @@
}
ArtifactNestedSetFunction.getInstance().updateValueForKey(key, value);
} catch (IOException e) {
- for (Artifact input : skyKeyToArtifactSet.get(key)) {
- handleIOException(input, e);
- }
+ handleIOExceptionFromSkykey(key, e);
} catch (ActionExecutionException e) {
- for (Artifact input : skyKeyToArtifactSet.get(key)) {
- handleActionExecutionException(input, e);
- }
+ handleActionExecutionExceptionFromSkykey(key, e);
} catch (ArtifactNestedSetEvalException e) {
for (Pair<SkyKey, Exception> skyKeyAndException : e.getNestedExceptions().toList()) {
SkyKey skyKey = skyKeyAndException.getFirst();
@@ -1601,13 +1600,12 @@
"Unexpected exception type: %s, key: %s",
inputException,
skyKey);
- for (Artifact input : skyKeyToArtifactSet.get(skyKey)) {
- if (inputException instanceof IOException) {
- handleIOException(input, (IOException) inputException);
- } else {
- handleActionExecutionException(input, (ActionExecutionException) inputException);
- }
+ if (inputException instanceof IOException) {
+ handleIOExceptionFromSkykey(skyKey, (IOException) inputException);
+ continue;
}
+ handleActionExecutionExceptionFromSkykey(
+ skyKey, (ActionExecutionException) inputException);
}
}
}
@@ -1615,6 +1613,26 @@
maybeThrowException();
}
+ private void handleActionExecutionExceptionFromSkykey(SkyKey key, ActionExecutionException e) {
+ if (key instanceof Artifact) {
+ handleActionExecutionExceptionPerArtifact((Artifact) key, e);
+ return;
+ }
+ for (Artifact input : skyKeyToDerivedArtifactSet.get(key)) {
+ handleActionExecutionExceptionPerArtifact(input, e);
+ }
+ }
+
+ private void handleIOExceptionFromSkykey(SkyKey key, IOException e) {
+ if (key instanceof Artifact) {
+ handleIOExceptionPerArtifact((Artifact) key, e);
+ return;
+ }
+ for (Artifact input : skyKeyToDerivedArtifactSet.get(key)) {
+ handleIOExceptionPerArtifact(input, e);
+ }
+ }
+
void accumulateMissingFileArtifactValue(Artifact input, MissingFileArtifactValue value) {
missingArtifactCauses.add(createLabelCause(input, value, action.getOwner().getLabel()));
}
@@ -1658,7 +1676,8 @@
|| mandatoryInputs.contains(input);
}
- private void handleActionExecutionException(Artifact input, ActionExecutionException e) {
+ private void handleActionExecutionExceptionPerArtifact(
+ Artifact input, ActionExecutionException e) {
if (isMandatory(input)) {
// Prefer a catastrophic exception as the one we propagate.
if (firstActionExecutionException == null
@@ -1669,7 +1688,7 @@
}
}
- private void handleIOException(Artifact input, IOException e) {
+ private void handleIOExceptionPerArtifact(Artifact input, IOException e) {
if (!input.isSourceArtifact()) {
BugReport.sendBugReport(
new IllegalStateException(
