Make include scanning faster when a target has many include scannables.

Instead of repeatedly scanning all transitively required include scannables, do
one pass from all include scannables; this prevents us revisiting the common
transitive closure of the include scannables.

Additionally, only scan command line includes relatively to a main source file,
not relatively to all source files. This is better than what we had before, but
it's still not exactly right - we should actually scan the command line
includes relatively to the module map file ([]

Brings include scanning times for large TUs with modules down from ~60 seconds
to < 2 seconds.

--
MOS_MIGRATED_REVID=88963159

3 files changed, 42 insertions, 25 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 23d841deba..5d2857a995 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
@@ -448,19 +448,26 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
     }
     return cmdlineIncludes.build();
   }
+  
+  @Override
+  public Artifact getMainIncludeScannerSource() {
+    return CppFileTypes.CPP_MODULE_MAP.matches(getSourceFile().getPath())
+        ? Iterables.getFirst(context.getHeaderModuleSrcs(), null)
+        : getSourceFile();
+  }
 
   @Override
   public Collection<Artifact> getIncludeScannerSources() {
     NestedSetBuilder<Artifact> builder = NestedSetBuilder.stableOrder();
     // For every header module we use for the build we need the set of sources that it can
     // reference.
-    builder.addAll(context.getTransitiveHeaderModuleSrcs());
+    builder.addTransitive(context.getTransitiveHeaderModuleSrcs());
     if (CppFileTypes.CPP_MODULE_MAP.matches(getSourceFile().getPath())) {
       // If this is an action that compiles the header module itself, the source we build is the
       // module map, and we need to include-scan all headers that are referenced in the module map.
       // We need to do include scanning as long as we want to support building code bases that are
       // not fully strict layering clean.
-      builder.addAll(context.getHeaderModuleSrcs());
+      builder.addTransitive(context.getHeaderModuleSrcs());
     } else {
       builder.add(getSourceFile());
     }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScannable.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScannable.java
index 9c70090ac0..d1f89f9657 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScannable.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScannable.java
@@ -68,8 +68,15 @@ public interface IncludeScannable {
   List<String> getCmdlineIncludes();
 
   /**
+   * Returns the artifact relative to which the {@code getCmdlineIncludes()} should be interpreted. 
+   */
+  Artifact getMainIncludeScannerSource();
+  
+  /**
    * Returns an immutable list of sources that the IncludeScanner should scan
    * for this action.
+   * 
+   * <p>Must contain {@code getMainIncludeScannerSource()}.
    */
   Collection<Artifact> getIncludeScannerSources();
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScanner.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScanner.java
index 65d89b9861..6929975569 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScanner.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScanner.java
@@ -45,15 +45,18 @@ import java.util.Set;
  */
 public interface IncludeScanner {
   /**
-   * Processes a source file and a list of includes extracted from command line
+   * Processes source files and a list of includes extracted from command line
    * flags. Adds all found files to the provided set {@code includes}. This
    * method takes into account the path- and file-level hints that are part of
    * this include scanner.
+   * 
+   * <p>{@code mainSource} is the source file relative to which the {@code cmdlineIncludes} are
+   * interpreted.
    */
-  public void process(Artifact source, Map<Artifact, Path> legalOutputPaths,
-      List<String> cmdlineIncludes, Set<Artifact> includes,
-      ActionExecutionContext actionExecutionContext)
-      throws IOException, ExecException, InterruptedException;
+  public void process(Artifact mainSource, Collection<Artifact> sources,
+      Map<Artifact, Path> legalOutputPaths, List<String> cmdlineIncludes, Set<Artifact> includes,
+      ActionExecutionContext actionExecutionContext) throws IOException, ExecException,
+      InterruptedException;
 
   /** Supplies IncludeScanners upon request. */
   interface IncludeScannerSupplier {
@@ -119,25 +122,25 @@ public interface IncludeScanner {
               relativeTo(execRoot, quoteIncludeDirs),
               relativeTo(execRoot, includeDirs));
 
-          for (Artifact source : scannable.getIncludeScannerSources()) {
-            // Add all include scanning entry points to the inputs; this is necessary
-            // when we have more than one source to scan from, for example when building
-            // C++ modules.
-            // In that case we have one of two cases:
-            // 1. We compile a header module - there, the .cppmap file is the main source file
-            //    (which we do not include-scan, as that would require an extra parser), and
-            //    thus already in the input; all headers in the .cppmap file are our entry points
-            //    for include scanning, but are not yet in the inputs - they get added here.
-            // 2. We compile an object file that uses a header module; currently using a header
-            //    module requires all headers it can reference to be available for the compilation.
-            //    The header module can reference headers that are not in the transitive include
-            //    closure of the current translation unit. Therefore, {@code CppCompileAction}
-            //    adds all headers specified transitively for compiled header modules as include
-            //    scanning entry points, and we need to add the entry points to the inputs here.
-            includes.add(source);
-            scanner.process(source, legalOutputPaths, cmdlineIncludes, includes,
+          Artifact mainSource =  scannable.getMainIncludeScannerSource();
+          Collection<Artifact> sources = scannable.getIncludeScannerSources();
+          // Add all include scanning entry points to the inputs; this is necessary
+          // when we have more than one source to scan from, for example when building
+          // C++ modules.
+          // In that case we have one of two cases:
+          // 1. We compile a header module - there, the .cppmap file is the main source file
+          //    (which we do not include-scan, as that would require an extra parser), and
+          //    thus already in the input; all headers in the .cppmap file are our entry points
+          //    for include scanning, but are not yet in the inputs - they get added here.
+          // 2. We compile an object file that uses a header module; currently using a header
+          //    module requires all headers it can reference to be available for the compilation.
+          //    The header module can reference headers that are not in the transitive include
+          //    closure of the current translation unit. Therefore, {@code CppCompileAction}
+          //    adds all headers specified transitively for compiled header modules as include
+          //    scanning entry points, and we need to add the entry points to the inputs here.
+          includes.addAll(sources);
+          scanner.process(mainSource, sources, legalOutputPaths, cmdlineIncludes, includes,
                 actionExecutionContext);
-          }
         }
       } catch (IOException e) {
         throw new EnvironmentalExecException(e.getMessage());