spawn action execution directly depends on runfiles artifacts

[benjaminp]

Back in 0.4.x and earlier, SpawnActions created by ctx.action always had an empty runfiles supplier. At some point—I think 4a87738—, such SpawnActions started having non-empty runfiles suppliers. As ActionExecutionFunction dutifully declares deps on all action inputs and runfiles supplier artifacts, one of the major reasons for runfiles middleman is defeated. This hits actions that have tools with many runfiles hard. In one large build I have, this bug increased the total number of skyframe edges by 10x (1.6 million to 17 million) leading to a major memory (and GC time) regression.

The simplest solution is to not depend on runfiles artifacts in ActionExecutionFunction. I.e.,

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 1b4bb2950..70c7ffe4b 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
@@ -231,8 +231,7 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
   @Nullable
   private AllInputs collectInputs(Action action, Environment env)
       throws ActionExecutionFunctionException, InterruptedException {
-    Iterable<Artifact> allKnownInputs = Iterables.concat(
-        action.getInputs(), action.getRunfilesSupplier().getArtifacts());
+    Iterable<Artifact> allKnownInputs = action.getInputs();
     if (action.inputsDiscovered()) {
       return new AllInputs(allKnownInputs);
     }

This appears to fix the problem. It also mostly passes tests; however, remote execution requires runfiles to be in the PerActionFileCache. I'm not sure if there's a good way around this. (Can we treat the runfiles middleman as an aggregating middleman and pull the transitive ArtifactValues through as this comment suggests?)

[philwo]

This seems like an area that @ulfjack might know something about.

[ulfjack]

@tomlu FYI

I don't have any immediate ideas on how to improve this. Maybe we need to do some special support in Skyframe?