Micro-optimize `Iterable` usage in `ActionExecutionFunction#addDiscoveredInputs`.
Problem: In practice, the implementation of `Environment#getOrderedValuesAndExceptions` iterates over its given `Iterable` 3 times. For the call in `ActionExecutionFunction#addDiscoveredInputs`, this `Iterable` is a lazy transformation of the one from `InputDiscoveryState#filterKnownDiscoveredInputs` which itself is a lazy filter using `ActionInputMap#getMetadata`. When there are a lot of inputs, this wasteful repeated work adds up.
Solution: Create a temporary `ImmutableList` formed by doing `InputDiscoveryState#filterKnownDiscoveredInputs` once, and use that for the call to `Environment#getOrderedValuesAndExceptions`. The theoretical downside is we potentially increase our peak heap usage. On a benchmark of extreme build with lots of actions with lots of input files, this approach consistently saves 2% CPU but has no significant impact on peak heap usage.
Alternatives considered: Refactor `Environment#getOrderedValuesAndExceptions` to reduce the number of iterations over the given `Iterable`. But given that both aggregate and specific CPU profiles show the heavy hitter source of the problem is `ActionExecutionFunction#addDiscoveredInputs` and also a general refactor might not reduce the number of iterations as well as this CL here, I decided against this approach.
PiperOrigin-RevId: 446464608
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 b1bf5c0..868671d 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
@@ -909,8 +909,15 @@
throws InterruptedException, ActionExecutionException {
// TODO(janakr): This code's assumptions are wrong in the face of Starlark actions with unused
// inputs, since ActionExecutionExceptions can come through here and should be aggregated. Fix.
+
+ // Environment#getOrderedValuesAndExceptions iterates over its given Iterable 3 times total.
+ // Since our discoveredInputs itself comes from InputDiscoveryState#filterKnownDiscoveredInputs,
+ // we create a single list once to avoid repeating the overhead of TransformedIterable and
+ // ActionInputMap#getMetadata.
+ ImmutableList<SkyKey> discoveredInputsAsArtifactKeys =
+ ImmutableList.copyOf(Iterables.transform(discoveredInputs, Artifact::key));
SkyframeIterableResult nonMandatoryDiscovered =
- env.getOrderedValuesAndExceptions(Iterables.transform(discoveredInputs, Artifact::key));
+ env.getOrderedValuesAndExceptions(discoveredInputsAsArtifactKeys);
if (!nonMandatoryDiscovered.hasNext()) {
return DiscoveredState.NO_DISCOVERED_DATA;
}
