Remove the need for discoverInputsStage2(). Much like every other Skyframe

function, discoverInputs() can be implemented to return null upon encountering
missing ActionExecutionValues.

RELNOTES: None.
PiperOrigin-RevId: 206913969

1 files changed, 20 insertions, 30 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
index 5b7c66faab..a73edf0697 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
@@ -140,9 +140,8 @@ public class CppCompileAction extends AbstractAction
   private Iterable<Artifact> additionalInputs = null;
 
   /**
-   * Set when a two-stage input discovery is used.
-   *
-   * <p>Used only during action execution.
+   * Used only during input discovery, when input discovery requires other actions
+   * to be executed first.
    */
   private Set<Artifact> usedModules = null;
 
@@ -160,13 +159,11 @@ public class CppCompileAction extends AbstractAction
    *   <li><i>Action caching.</i> It is set when restoring from the action cache. It is queried
    *       immediately after restoration to populate the {@link
    *       com.google.devtools.build.lib.skyframe.ActionExecutionValue}.
-   *   <li><i>Input discovery</i>It is set by {@link #discoverInputsStage2}. It is queried to
+   *   <li><i>Input discovery</i>It is set by {@link #discoverInputs}. It is queried to
    *       populate the {@link com.google.devtools.build.lib.skyframe.ActionExecutionValue}.
    *   <li><i>Compilation</i>Compilation reads this field to know what needs to be staged.
    * </ul>
    */
-  // TODO(djasper): investigate releasing memory used by this field as early as possible, for
-  // example, by including these values in additionalInputs.
   private ImmutableList<Artifact> discoveredModules = null;
 
   /**
@@ -439,45 +436,38 @@ public class CppCompileAction extends AbstractAction
       throws ActionExecutionException, InterruptedException {
     Preconditions.checkArgument(!sourceFile.isFileType(CppFileTypes.CPP_MODULE));
 
-    additionalInputs = findUsedHeaders(actionExecutionContext);
-    if (!shouldScanIncludes) {
-      return additionalInputs;
-    }
+    if (additionalInputs == null) {
+      additionalInputs = findUsedHeaders(actionExecutionContext);
 
-    if (!shouldScanDotdFiles()) {
-      additionalInputs = filterDiscoveredHeaders(actionExecutionContext, additionalInputs);
+      if (!shouldScanIncludes) {
+        return additionalInputs;
+      }
+
+      if (!shouldScanDotdFiles()) {
+        // If we aren't looking at .d files later, remove undeclared inputs now.
+        additionalInputs = filterDiscoveredHeaders(actionExecutionContext, additionalInputs);
+      }
     }
 
-    if (!shouldPruneModules) {
+    if (!shouldScanIncludes || !shouldPruneModules) {
       return additionalInputs;
     }
 
-    usedModules =
-        ccCompilationContext.getUsedModules(usePic, ImmutableSet.copyOf(additionalInputs));
-    return Iterables.concat(additionalInputs, usedModules);
-  }
-
-  /** @return null when either {@link #usedModules} was null or on Skyframe lookup failure */
-  @Nullable
-  @Override
-  public Iterable<Artifact> discoverInputsStage2(SkyFunction.Environment env)
-      throws InterruptedException {
     if (usedModules == null) {
-      // No modules were used in this compilation, no need to do any work.
-      return null;
+      usedModules =
+          ccCompilationContext.getUsedModules(usePic, ImmutableSet.copyOf(additionalInputs));
     }
-
-    Set<Artifact> transitivelyUsedModules = computeTransitivelyUsedModules(env, usedModules);
+    Set<Artifact> transitivelyUsedModules =
+        computeTransitivelyUsedModules(
+            actionExecutionContext.getEnvironmentForDiscoveringInputs(), usedModules);
     if (transitivelyUsedModules == null) {
-      // Not all used modules available yet. ActionExecutionValues have been requested. Return so
-      // that this function can be re-executed when ready.
       return null;
     }
 
     discoveredModules = ImmutableList.copyOf(Sets.union(usedModules, transitivelyUsedModules));
     topLevelModules = ImmutableList.copyOf(Sets.difference(usedModules, transitivelyUsedModules));
     usedModules = null;
-    return transitivelyUsedModules;
+    return Iterables.concat(additionalInputs, discoveredModules);
   }
 
   @Override