Add experimental flag to stop requiring all transitive modules as inputs.

Requiring all transitive modules to always be available can lead to long
critical paths and even unnecessary compiles in combination with the
prune_header_modules feature.

--
MOS_MIGRATED_REVID=137696794

6 files changed, 86 insertions, 7 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
index 85ea16dd42..a20888b8f2 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
@@ -175,6 +175,12 @@ public abstract class AbstractAction implements Action, SkylarkValue {
 
   @Nullable
   @Override
+  public Iterable<Artifact> getInputsWhenSkippingInputDiscovery() {
+    return null;
+  }
+
+  @Nullable
+  @Override
   public Iterable<Artifact> resolveInputsFromCache(
       ArtifactResolver artifactResolver,
       PackageRootResolver resolver,
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Action.java b/src/main/java/com/google/devtools/build/lib/actions/Action.java
index 54b3d8634d..ecf8b66182 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Action.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Action.java
@@ -152,6 +152,32 @@ public interface Action extends ActionExecutionMetadata, Describable {
       throws ActionExecutionException, InterruptedException;
 
   /**
+   * If an action does not know the exact set of inputs ahead of time, it will usually either do:
+   * <ul>
+   * <li> Execution time pruning: The action provides a superset set of inputs at action creation
+   *      time, and prunes inputs during {@link #execute}.
+   * <li> Input discovery: The action provides a subset of inputs at creation time, overwrites
+   *      {@link #discoverInputs} to return a superset of inputs depending on the execution context
+   *      and optionally prunes the inputs at the end of {@link #execute} to a required subset for
+   *      subsequent calls.
+   * </ul>
+   *
+   * This function allows an action that is set up for input discovery, and thus only provides a
+   * minimal set of inputs at creation time, to switch off input discovery depending on the
+   * execution context. To that end the action must:
+   * <ul>
+   * <li>Provide a subset of inputs at creation time
+   * <li>Return {@code null} from {@link #discoverInputs}, indicating no input discovery
+   * <li>Return a superset of inputs that might have been discovered otherwise from here;
+   *     this will usually be the set difference between the full set of inputs the action would get
+   *     when doing only execution time pruning and the minimal subset provided above.
+   * <li>Prune the set of inputs on execute
+   * </ul>
+   */
+  @Nullable
+  Iterable<Artifact> getInputsWhenSkippingInputDiscovery();
+
+  /**
    * Method used to resolve action inputs based on the information contained in the action cache. It
    * will be called iff inputsKnown() is false for the given action instance and there is a related
    * cache entry in the action cache.
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 ad24a7d95a..96e93da3a9 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
@@ -233,7 +233,6 @@ public class CppCompileAction extends AbstractAction
    * @param dwoFile the .dwo output file where debug information is stored for Fission builds (null
    *     if Fission mode is disabled)
    * @param optionalSourceFile an additional optional source file (null if unneeded)
-   * @param configuration the build configurations
    * @param cppConfiguration TODO(bazel-team): Add parameter description.
    * @param context the compilation context
    * @param actionContext TODO(bazel-team): Add parameter description.
@@ -290,7 +289,9 @@ public class CppCompileAction extends AbstractAction
             ruleContext,
             mandatoryInputs,
             context.getTransitiveCompilationPrerequisites(),
-            context.getUseHeaderModules() ? context.getTransitiveModules(usePic) : null,
+            context.getUseHeaderModules() && !cppConfiguration.getSkipUnusedModules()
+                ? context.getTransitiveModules(usePic)
+                : null,
             optionalSourceFile,
             lipoScannables),
         CollectionUtils.asListWithoutNulls(
@@ -355,7 +356,6 @@ public class CppCompileAction extends AbstractAction
       }
 
       // One starting ../ is okay for getting to a sibling repository.
-      PathFragment originalInclude = include;
       if (include.startsWith(new PathFragment(Label.EXTERNAL_PATH_PREFIX))) {
         include = include.relativeTo(Label.EXTERNAL_PATH_PREFIX);
       }
@@ -430,10 +430,6 @@ public class CppCompileAction extends AbstractAction
     return builtinIncludeFiles;
   }
 
-  public List<Artifact> getadditionalIncludeScannables() {
-    return additionalIncludeScannables;
-  }
-
   public String getHostSystemName() {
     return cppConfiguration.getHostSystemName();
   }
@@ -444,6 +440,21 @@ public class CppCompileAction extends AbstractAction
   }
 
   @Override
+  public ImmutableSet<Artifact> getMandatoryOutputs() {
+    // Never prune orphaned modules files. To cut down critical paths, CppCompileActions do not
+    // add modules files as inputs. Instead they rely on input discovery to recognize the needed
+    // ones. However, orphan detection runs before input discovery and thus module files would be
+    // discarded as orphans.
+    // This is strictly better than marking all transitive modules as inputs, which would also
+    // effectively disable orphan detection for .pcm files.
+    if (cppConfiguration.getSkipUnusedModules()
+        && CppFileTypes.CPP_MODULE.matches(outputFile.getFilename())) {
+      return ImmutableSet.of(outputFile);
+    }
+    return super.getMandatoryOutputs();
+  }
+
+  @Override
   public synchronized boolean inputsKnown() {
     return inputsKnown;
   }
@@ -521,6 +532,15 @@ public class CppCompileAction extends AbstractAction
   }
 
   @Override
+  public Iterable<Artifact> getInputsWhenSkippingInputDiscovery() {
+    if (context.getUseHeaderModules()
+        && cppConfiguration.getSkipUnusedModules()) {
+      return context.getTransitiveModules(usePic);
+    }
+    return null;
+  }
+
+  @Override
   public Artifact getPrimaryInput() {
     return getSourceFile();
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
index 2b839de920..5a43a716c2 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
@@ -1763,6 +1763,10 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
     return cppOptions.inmemoryDotdFiles;
   }
 
+  public boolean getSkipUnusedModules() {
+    return cppOptions.skipUnusedModules;
+  }
+
   public LibcTop getLibcTop() {
     return cppOptions.libcTop;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
index edf780dd76..becf653502 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
@@ -527,6 +527,16 @@ public class CppOptions extends FragmentOptions {
   public boolean inmemoryDotdFiles;
 
   @Option(
+    name = "experimental_skip_unused_modules",
+    defaultValue = "false",
+    category = "experimental",
+    help =
+        "If enabled, not all transitive modules automatically become an action's inputs. Instead,"
+            + " input discovery adds just the required ones."
+  )
+  public boolean skipUnusedModules;
+
+  @Option(
     name = "experimental_omitfp",
     defaultValue = "false",
     category = "semantics",
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 a21caa8fa9..7e07b3c4da 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
@@ -403,6 +403,19 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
           perActionFileCache = new PerActionFileCache(state.inputArtifactData);
           metadataHandler =
               new ActionMetadataHandler(state.inputArtifactData, action.getOutputs(), tsgm.get());
+        } else {
+          // The action generally tries to discover its inputs during execution. If there are any
+          // additional inputs necessary to execute the action, make sure they are available now.
+          Iterable<Artifact> requiredInputs = action.getInputsWhenSkippingInputDiscovery();
+          if (requiredInputs != null) {
+            addDiscoveredInputs(state.inputArtifactData, requiredInputs, env);
+            if (env.valuesMissing()) {
+              return null;
+            }
+            perActionFileCache = new PerActionFileCache(state.inputArtifactData);
+            metadataHandler =
+                new ActionMetadataHandler(state.inputArtifactData, action.getOutputs(), tsgm.get());
+          }
         }
       }
       actionExecutionContext =