In ActionExecutionFunction, delay construction of the skykey->artifact multimap used only for error processing until we know that it's required. As a result, don't unroll the all-inputs nested set until after we know all deps are present. Saves nested-set unrolling in the missing-dep+error-free case (up to 1/2 of all action executions) and saves iteration and multimap construction in the error-free case (~all actions). PiperOrigin-RevId: 366845997
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 1c2bfd5..e9091ee 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
@@ -19,6 +19,7 @@ import com.google.common.base.Joiner; import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; +import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -111,6 +112,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.function.IntFunction; +import java.util.function.Supplier; import javax.annotation.Nullable; import net.starlark.java.eval.StarlarkSemantics; @@ -1339,23 +1341,23 @@ AccumulateInputResultsFactory<S, R> accumulateInputResultsFactory, boolean allowValuesMissingEarlyReturn) throws ActionExecutionException, InterruptedException { - ImmutableList<Artifact> allInputsList = allInputs.toList(); - - // Some keys have more than 1 corresponding Artifact (e.g. actions with 2 outputs). - // For Artifacts whose Artifact::key isn't itself. - Multimap<SkyKey, Artifact> skyKeyToArtifactSet = - MultimapBuilder.hashKeys().hashSetValues().build(); - allInputsList.forEach( - input -> { - SkyKey key = Artifact.key(input); - if (key != input) { - skyKeyToArtifactSet.put(key, input); - } - }); ActionExecutionFunctionExceptionHandler actionExecutionFunctionExceptionHandler = new ActionExecutionFunctionExceptionHandler( - skyKeyToArtifactSet, + Suppliers.memoize( + () -> { + ImmutableList<Artifact> allInputsList = allInputs.toList(); + Multimap<SkyKey, Artifact> skyKeyToArtifactSet = + MultimapBuilder.hashKeys().hashSetValues().build(); + allInputsList.forEach( + input -> { + SkyKey key = Artifact.key(input); + if (key != input) { + skyKeyToArtifactSet.put(key, input); + } + }); + return skyKeyToArtifactSet; + }), inputDeps, action, mandatoryInputs, @@ -1369,6 +1371,8 @@ return null; } + ImmutableList<Artifact> allInputsList = allInputs.toList(); + // When there are no missing values or there was an error, we can start checking individual // files. We don't bother to optimize the error-ful case since it's rare. Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesetsInsideRunfiles = @@ -1591,7 +1595,7 @@ /** Helper subclass for the error-handling logic for ActionExecutionFunction#accumulateInputs. */ private final class ActionExecutionFunctionExceptionHandler { - private final Multimap<SkyKey, Artifact> skyKeyToDerivedArtifactSet; + private final Supplier<Multimap<SkyKey, Artifact>> skyKeyToDerivedArtifactSetForExceptions; private final List< ValueOrException3< SourceArtifactException, ActionExecutionException, ArtifactNestedSetEvalException>> @@ -1605,7 +1609,7 @@ private ActionExecutionException firstActionExecutionException; ActionExecutionFunctionExceptionHandler( - Multimap<SkyKey, Artifact> skyKeyToDerivedArtifactSet, + Supplier<Multimap<SkyKey, Artifact>> skyKeyToDerivedArtifactSetForExceptions, List< ValueOrException3< SourceArtifactException, @@ -1616,7 +1620,7 @@ Set<Artifact> mandatoryInputs, Iterable<SkyKey> requestedSkyKeys, boolean valuesMissing) { - this.skyKeyToDerivedArtifactSet = skyKeyToDerivedArtifactSet; + this.skyKeyToDerivedArtifactSetForExceptions = skyKeyToDerivedArtifactSetForExceptions; this.inputDeps = inputDeps; this.action = action; this.mandatoryInputs = mandatoryInputs; @@ -1688,7 +1692,7 @@ handleActionExecutionExceptionPerArtifact((Artifact) key, e); return; } - for (Artifact input : skyKeyToDerivedArtifactSet.get(key)) { + for (Artifact input : skyKeyToDerivedArtifactSetForExceptions.get().get(key)) { handleActionExecutionExceptionPerArtifact(input, e); } }