Declare dependencies on discovered inputs before execution instead of after.

As a side effect, we no longer restart ActionExecutionFunction after the action has executed unless the action ran locally (and therefore didn't discover its inputs beforehand). This also means that we no longer need to store an out-of-Skyframe cache for discovered includes except when checking the action cache. Since my suspicion is that the out-of-Skyframe cache will have a minimal performance impact once it is just being used for the action cache, I may delete it in a follow-up cl.

After this change, we will overapproximate the set of includes because we depend on all includes, rather than just the ones that the action was found to depend on after execution.

This is a prerequisite for Skyframe-native include scanning in that Skyframe-native include scanning will need to add at least as many Skyframe nodes and edges. If we end up punting on it, then we may want to revert this change. But for now I think it's worth having. I'll run some more numbers to see what the actual performance impact is.

--
MOS_MIGRATED_REVID=88492955

1 files changed, 6 insertions, 2 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index 5bf35799b5..2e76c14927 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -206,6 +206,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
   private final AtomicReference<ActionExecutionStatusReporter> statusReporterRef =
       new AtomicReference<>();
   private final SkyframeActionExecutor skyframeActionExecutor;
+  private CompletionReceiver actionExecutionFunction;
   protected SkyframeProgressReceiver progressReceiver;
   private final AtomicReference<CyclesReporter> cyclesReporter = new AtomicReference<>();
 
@@ -320,8 +321,10 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
         buildDataDirectory));
     map.put(SkyFunctions.BUILD_INFO, new WorkspaceStatusFunction());
     map.put(SkyFunctions.COVERAGE_REPORT, new CoverageReportFunction());
-    map.put(SkyFunctions.ACTION_EXECUTION,
-        new ActionExecutionFunction(skyframeActionExecutor, tsgm));
+    ActionExecutionFunction actionExecutionFunction =
+        new ActionExecutionFunction(skyframeActionExecutor, tsgm);
+    map.put(SkyFunctions.ACTION_EXECUTION, actionExecutionFunction);
+    this.actionExecutionFunction = actionExecutionFunction;
     map.put(SkyFunctions.RECURSIVE_FILESYSTEM_TRAVERSAL,
         new RecursiveFilesystemTraversalFunction());
     map.put(SkyFunctions.FILESET_ENTRY, new FilesetEntryFunction());
@@ -955,6 +958,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
       // Also releases thread locks.
       resourceManager.resetResourceUsage();
       skyframeActionExecutor.executionOver();
+      actionExecutionFunction.complete();
     }
   }