Improve implementation of filterDiscoveredHeaders. Instead of creating a huge

set of all declared headers, iterate over them and remove them from a (often
much smaller) set.

This should improve runtime as well as amount of garbage produced.

RELNOTES: None.
PiperOrigin-RevId: 207710867

1 files changed, 32 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 fd9ac8e36b..d8fb87d8ab 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
@@ -387,46 +387,48 @@ public class CppCompileAction extends AbstractAction
    */
   private Iterable<Artifact> filterDiscoveredHeaders(
       ActionExecutionContext actionExecutionContext, Iterable<Artifact> headers) {
-    // Get the inputs we know about. Note that this (compared to validateInclusions) does not
-    // take mandatoryInputs into account. The reason is that these by definition get added to the
-    // action input and thus are available anyway. Not having to look at them here saves us from
-    // requiring and ArtifactExpander, which actionExecutionContext doesn't have at this point.
-    // This only works as long as mandatory inputs do not contain headers that are built into a
-    // module.
-    Set<Artifact> allowedIncludes =
-        new HashSet<>(ccCompilationContext.getDeclaredIncludeSrcs().toCollection());
-    allowedIncludes.addAll(additionalPrunableHeaders.toCollection());
-
-    // Whitelisted directories. Lazily initialize, so that compiles that properly declare all
-    // their files profit.
-    Set<PathFragment> declaredIncludeDirs = null;
-    Iterable<PathFragment> ignoreDirs = null;
+    Set<Artifact> undeclaredHeaders = Sets.newHashSet(headers);
+
+    // Note that this (compared to validateInclusions) does not take mandatoryInputs into account.
+    // The reason is that these by definition get added to the action input and thus are available
+    // anyway. Not having to look at them here saves us from requiring and ArtifactExpander, which
+    // actionExecutionContext doesn't have at this point. This only works as long as mandatory
+    // inputs do not contain headers that are built into a module.
+    for (Artifact header : ccCompilationContext.getDeclaredIncludeSrcs()) {
+      undeclaredHeaders.remove(header);
+    }
+    for (Artifact header : additionalPrunableHeaders) {
+      undeclaredHeaders.remove(header);
+    }
+    if (undeclaredHeaders.isEmpty()) {
+      return headers;
+    }
 
-    // Create a filtered list of headers.
-    ImmutableList.Builder<Artifact> result = ImmutableList.builder();
-    for (Artifact header : headers) {
-      if (allowedIncludes.contains(header)) {
-        result.add(header);
-        continue;
-      }
-      if (ignoreDirs == null) {
-        ignoreDirs =
-            cppConfiguration.isStrictSystemIncludes()
-                ? getBuiltInIncludeDirectories()
-                : getValidationIgnoredDirs();
-      }
+    Iterable<PathFragment> ignoreDirs =
+        cppConfiguration.isStrictSystemIncludes()
+            ? getBuiltInIncludeDirectories()
+            : getValidationIgnoredDirs();
+    ArrayList<Artifact> found = new ArrayList<>();
+    // Lazily initialize, so that compiles that properly declare all their files profit.
+    Set<PathFragment> declaredIncludeDirs = null;
+    for (Artifact header : undeclaredHeaders) {
       if (FileSystemUtils.startsWithAny(header.getExecPath(), ignoreDirs)) {
-        result.add(header);
+        found.add(header);
         continue;
       }
       if (declaredIncludeDirs == null) {
         declaredIncludeDirs = ccCompilationContext.getDeclaredIncludeDirs().toSet();
       }
       if (isDeclaredIn(actionExecutionContext, header, declaredIncludeDirs)) {
-        result.add(header);
+        found.add(header);
       }
     }
-    return result.build();
+    undeclaredHeaders.removeAll(found);
+    if (undeclaredHeaders.isEmpty()) {
+      return headers;
+    }
+
+    return Iterables.filter(headers, header -> !undeclaredHeaders.contains(header));
   }
 
   @Nullable