Track additionallyPrunableIncludes separately from declaredIncludeSrcs (don't

compose a nested set encompassing both). The current prunableHeaders were used
in various places, at least two of which let to a duplicate iteration through
the comparatively large declaredIncludeSrcs: CppCompileAction.computeKey() and
CppCompileAction.getAllowedDerivedInputs().

Also do some other minor optimizations:
- CcCompileActionBuilder.buildAllInputs() now just returns a NestedSet with the
  same order as its parameter. As inputsForValidation is almost always empty,
  this makes this NestedSet pass the underlying one through.
- CcCompilationContext.directModuleMaps is just a list of the module maps of
  direct dependencies. As such, there is no use for keeping them in a
  NestedSet.

RELNOTES: None.
PiperOrigin-RevId: 203938011

3 files changed, 39 insertions, 39 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java
index e5bc2f85fe..593ecd212f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java
@@ -60,10 +60,8 @@ public final class CcCompilationContext implements CcCompilationContextApi {
   private final NestedSet<PathFragment> declaredIncludeWarnDirs;
   private final NestedSet<Artifact> declaredIncludeSrcs;
 
-  /**
-   * Module maps from direct dependencies.
-   */
-  private final NestedSet<Artifact> directModuleMaps;
+  /** Module maps from direct dependencies. */
+  private final ImmutableList<Artifact> directModuleMaps;
 
   /** Non-code mandatory compilation inputs. */
   private final NestedSet<Artifact> nonCodeInputs;
@@ -94,7 +92,7 @@ public final class CcCompilationContext implements CcCompilationContextApi {
       NestedSet<HeaderInfo> transitiveHeaderInfos,
       NestedSet<Artifact> transitiveModules,
       NestedSet<Artifact> transitivePicModules,
-      NestedSet<Artifact> directModuleMaps,
+      ImmutableList<Artifact> directModuleMaps,
       CppModuleMap cppModuleMap,
       @Nullable CppModuleMap verificationModuleMap,
       boolean propagateModuleMapAsActionInput) {
@@ -265,7 +263,7 @@ public final class CcCompilationContext implements CcCompilationContextApi {
    */
   public NestedSet<Artifact> getAdditionalInputs() {
     NestedSetBuilder<Artifact> builder = NestedSetBuilder.stableOrder();
-    builder.addTransitive(directModuleMaps);
+    builder.addAll(directModuleMaps);
     builder.addTransitive(nonCodeInputs);
     if (cppModuleMap != null && propagateModuleMapAsActionInput) {
       builder.add(cppModuleMap.getArtifact());
@@ -273,10 +271,8 @@ public final class CcCompilationContext implements CcCompilationContextApi {
     return builder.build();
   }
 
-  /**
-   * @return modules maps from direct dependencies.
-   */
-  public NestedSet<Artifact> getDirectModuleMaps() {
+  /** @return modules maps from direct dependencies. */
+  public Iterable<Artifact> getDirectModuleMaps() {
     return directModuleMaps;
   }
 
@@ -378,7 +374,7 @@ public final class CcCompilationContext implements CcCompilationContextApi {
         NestedSetBuilder.stableOrder();
     private final NestedSetBuilder<Artifact> transitiveModules = NestedSetBuilder.stableOrder();
     private final NestedSetBuilder<Artifact> transitivePicModules = NestedSetBuilder.stableOrder();
-    private final NestedSetBuilder<Artifact> directModuleMaps = NestedSetBuilder.stableOrder();
+    private final Set<Artifact> directModuleMaps = new LinkedHashSet<>();
     private final Set<String> defines = new LinkedHashSet<>();
     private CppModuleMap cppModuleMap;
     private CppModuleMap verificationModuleMap;
@@ -685,7 +681,7 @@ public final class CcCompilationContext implements CcCompilationContextApi {
           transitiveHeaderInfo.build(),
           transitiveModules.build(),
           transitivePicModules.build(),
-          directModuleMaps.build(),
+          ImmutableList.copyOf(directModuleMaps),
           cppModuleMap,
           verificationModuleMap,
           propagateModuleMapAsActionInput);
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 1881fb7853..372e7653a7 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
@@ -99,15 +99,11 @@ public class CppCompileAction extends AbstractAction
   private final Iterable<Artifact> inputsForInvalidation;
 
   /**
-   * The set of input files that we add to the set of input artifacts of the action if we don't use
-   * input discovery. They may be pruned after execution.
-   *
-   * <p>This is necessary because the inputs that can be pruned by .d file parsing must be returned
-   * from {@link #discoverInputs(ActionExecutionContext)} and they cannot be in
-   * {@link #mandatoryInputs}. Thus, even with include scanning turned off, we pretend that we
-   * "discover" these headers.
+   * The set of input files that in addition to {@link CcCompilationContext#declaredIncludeSrcs}
+   * need to be added to the set of input artifacts of the action if we don't use input discovery.
+   * They may be pruned after execution. See {@link #findUsedHeaders} for more details.
    */
-  private final NestedSet<Artifact> prunableHeaders;
+  private final NestedSet<Artifact> additionalPrunableHeaders;
 
   @Nullable private final Artifact grepIncludes;
   private final boolean shouldScanIncludes;
@@ -224,7 +220,7 @@ public class CppCompileAction extends AbstractAction
       NestedSet<Artifact> mandatoryInputs,
       Iterable<Artifact> inputsForInvalidation,
       ImmutableList<Artifact> builtinIncludeFiles,
-      NestedSet<Artifact> prunableHeaders,
+      NestedSet<Artifact> additionalPrunableHeaders,
       Artifact outputFile,
       DotdFile dotdFile,
       @Nullable Artifact gcnoFile,
@@ -258,7 +254,7 @@ public class CppCompileAction extends AbstractAction
     // definitely exist prior to this action execution.
     this.mandatoryInputs = mandatoryInputs;
     this.inputsForInvalidation = inputsForInvalidation;
-    this.prunableHeaders = prunableHeaders;
+    this.additionalPrunableHeaders = additionalPrunableHeaders;
     // inputsKnown begins as the logical negation of shouldScanIncludes.
     // When scanning includes, the inputs begin as not known, and become
     // known after inclusion scanning. When *not* scanning includes,
@@ -353,11 +349,17 @@ public class CppCompileAction extends AbstractAction
   @Override
   @VisibleForTesting // productionVisibility = Visibility.PRIVATE
   public Iterable<Artifact> getPossibleInputsForTesting() {
-    return Iterables.concat(getInputs(), prunableHeaders);
+    return Iterables.concat(
+        getInputs(), ccCompilationContext.getDeclaredIncludeSrcs(), additionalPrunableHeaders);
   }
 
   /**
    * Returns the results of include scanning or, when that is null, all prunable headers.
+   *
+   * <p>This is necessary because the inputs that can be pruned by .d file parsing must be returned
+   * from {@link #discoverInputs(ActionExecutionContext)} and they cannot be in {@link
+   * #mandatoryInputs}. Thus, even with include scanning turned off, we pretend that we "discover"
+   * these headers.
    */
   private Iterable<Artifact> findUsedHeaders(ActionExecutionContext actionExecutionContext)
       throws ActionExecutionException, InterruptedException {
@@ -373,8 +375,12 @@ public class CppCompileAction extends AbstractAction
           actionExecutionContext.getVerboseFailures(),
           this);
     }
-
-    return includeScanningResult == null ? prunableHeaders : includeScanningResult;
+    if (includeScanningResult != null) {
+      return includeScanningResult;
+    }
+    return NestedSetBuilder.fromNestedSet(ccCompilationContext.getDeclaredIncludeSrcs())
+        .addTransitive(additionalPrunableHeaders)
+        .build();
   }
 
   @Nullable
@@ -685,7 +691,11 @@ public class CppCompileAction extends AbstractAction
     IncludeProblems errors = new IncludeProblems();
     IncludeProblems warnings = new IncludeProblems();
     Set<Artifact> allowedIncludes = new HashSet<>();
-    for (Artifact input : Iterables.concat(mandatoryInputs, prunableHeaders)) {
+    for (Artifact input :
+        Iterables.concat(
+            mandatoryInputs,
+            ccCompilationContext.getDeclaredIncludeSrcs(),
+            additionalPrunableHeaders)) {
       if (input.isMiddlemanArtifact() || input.isTreeArtifact()) {
         actionExecutionContext.getArtifactExpander().expand(input, allowedIncludes);
       }
@@ -873,9 +883,8 @@ public class CppCompileAction extends AbstractAction
   public Iterable<Artifact> getAllowedDerivedInputs() {
     HashSet<Artifact> result = new HashSet<>();
     addNonSources(result, mandatoryInputs);
-    addNonSources(result, prunableHeaders);
+    addNonSources(result, additionalPrunableHeaders);
     addNonSources(result, getDeclaredIncludeSrcs());
-    addNonSources(result, ccCompilationContext.getTransitiveCompilationPrerequisites());
     addNonSources(result, ccCompilationContext.getTransitiveModules(usePic));
     Artifact artifact = getSourceFile();
     if (!artifact.isSourceArtifact()) {
@@ -977,7 +986,7 @@ public class CppCompileAction extends AbstractAction
     fp.addInt(0); // mark the boundary between input types
     actionKeyContext.addNestedSetToFingerprint(fp, getMandatoryInputs());
     fp.addInt(0);
-    actionKeyContext.addNestedSetToFingerprint(fp, prunableHeaders);
+    actionKeyContext.addNestedSetToFingerprint(fp, additionalPrunableHeaders);
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
index de0801c57e..a87df83677 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
@@ -284,12 +284,10 @@ public class CppCompileActionBuilder {
     NestedSet<Artifact> realMandatoryInputs = buildMandatoryInputs();
     NestedSet<Artifact> allInputs = buildAllInputs(realMandatoryInputs);
 
-    NestedSetBuilder<Artifact> prunableHeadersBuilder = NestedSetBuilder.stableOrder();
-    prunableHeadersBuilder.addTransitive(ccCompilationContext.getDeclaredIncludeSrcs());
-    prunableHeadersBuilder.addTransitive(cppSemantics.getAdditionalPrunableIncludes());
-    prunableHeadersBuilder.addAll(additionalPrunableHeaders);
-
-    NestedSet<Artifact> prunableHeaders = prunableHeadersBuilder.build();
+    NestedSet<Artifact> prunableHeaders =
+        NestedSetBuilder.fromNestedSet(cppSemantics.getAdditionalPrunableIncludes())
+            .addAll(additionalPrunableHeaders)
+            .build();
 
     // Copying the collections is needed to make the builder reusable.
     CppCompileAction action;
@@ -393,10 +391,7 @@ public class CppCompileActionBuilder {
    * Returns the list of all inputs for the {@link CppCompileAction} as configured.
    */
   NestedSet<Artifact> buildAllInputs(NestedSet<Artifact> mandatoryInputs) {
-    NestedSetBuilder<Artifact> builder = NestedSetBuilder.stableOrder();
-    builder.addTransitive(mandatoryInputs);
-    builder.addAll(inputsForInvalidation);
-    return builder.build();
+    return NestedSetBuilder.fromNestedSet(mandatoryInputs).addAll(inputsForInvalidation).build();
   }
 
   private boolean useHeaderModules() {