From 5ec54c91ff775cb50ac3b1b11ba0029a8c989359 Mon Sep 17 00:00:00 2001
From: Googler <noreply@google.com>
Date: Wed, 11 Jul 2018 06:04:30 -0700
Subject: Skip fingerprinting stuff into a CppCompileAction's key that can only
 affect the result of include validation if dotd file scanning (and in turn
 input validation) is disabled. Fingerprinting these data structures is costly
 as they are large NestedSets.

RELNOTES: None.
PiperOrigin-RevId: 204112075
---
 .../build/lib/rules/cpp/CppCompileAction.java      | 32 ++++++++++++++--------
 .../lib/rules/cpp/CppCompileActionBuilder.java     |  4 ++-
 2 files changed, 23 insertions(+), 13 deletions(-)

(limited to 'src/main/java/com/google/devtools')

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 372e7653a7..dfa2905bee 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
@@ -302,6 +302,12 @@ public class CppCompileAction extends AbstractAction
     return shouldScanIncludes;
   }
 
+  private boolean shouldScanDotdFiles() {
+    return !cppConfiguration.getNoDotdScanningWithModules()
+        || !useHeaderModules
+        || !shouldPruneModules;
+  }
+
   @Override
   public List<PathFragment> getBuiltInIncludeDirectories() {
     return builtInIncludeDirectories;
@@ -972,21 +978,23 @@ public class CppCompileAction extends AbstractAction
     // A better long-term solution would be to make the compiler to find them automatically and
     // never hand in the .pcm files explicitly on the command line in the first place.
     fp.addStrings(compileCommandLine.getArguments(/* overwrittenVariables= */ null));
-
-    /*
-     * getArguments() above captures all changes which affect the compilation
-     * command and hence the contents of the object file.  But we need to
-     * also make sure that we reexecute the action if any of the fields
-     * that affect whether validateIncludes() will report an error or warning
-     * have changed, otherwise we might miss some errors.
-     */
-    fp.addPaths(ccCompilationContext.getDeclaredIncludeDirs());
-    fp.addPaths(ccCompilationContext.getDeclaredIncludeWarnDirs());
     actionKeyContext.addNestedSetToFingerprint(fp, ccCompilationContext.getDeclaredIncludeSrcs());
     fp.addInt(0); // mark the boundary between input types
     actionKeyContext.addNestedSetToFingerprint(fp, getMandatoryInputs());
     fp.addInt(0);
     actionKeyContext.addNestedSetToFingerprint(fp, additionalPrunableHeaders);
+
+    fp.addBoolean(shouldScanDotdFiles());
+    if (shouldScanDotdFiles()) {
+      /**
+       * getArguments() above captures all changes which affect the compilation command and hence
+       * the contents of the object file. But we need to also make sure that we re-execute the
+       * action if any of the fields that affect whether {@link #validateInclusions} will report an
+       * error or warning have changed, otherwise we might miss some errors.
+       */
+      fp.addPaths(ccCompilationContext.getDeclaredIncludeDirs());
+      fp.addPaths(ccCompilationContext.getDeclaredIncludeWarnDirs());
+    }
   }
 
   @Override
@@ -1006,7 +1014,7 @@ public class CppCompileAction extends AbstractAction
       actionExecutionContext.getFileOutErr().setErrorFilter(showIncludesFilterForStderr);
     }
 
-    if (cppConfiguration.getNoDotdScanningWithModules() && useHeaderModules && shouldPruneModules) {
+    if (!shouldScanDotdFiles()) {
       updateActionInputs(
           NestedSetBuilder.wrap(
               Order.STABLE_ORDER, Iterables.concat(discoveredModules, additionalInputs)));
@@ -1030,7 +1038,7 @@ public class CppCompileAction extends AbstractAction
     }
     ensureCoverageNotesFilesExist(actionExecutionContext);
 
-    if (cppConfiguration.getNoDotdScanningWithModules() && useHeaderModules && shouldPruneModules) {
+    if (!shouldScanDotdFiles()) {
       return ActionResult.create(spawnResults);
     }
 
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 a87df83677..5c7d80b744 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
@@ -646,8 +646,10 @@ public class CppCompileActionBuilder {
     return this;
   }
 
-  public void setAdditionalPrunableHeaders(Iterable<Artifact> additionalPrunableHeaders) {
+  public CppCompileActionBuilder setAdditionalPrunableHeaders(
+      Iterable<Artifact> additionalPrunableHeaders) {
     this.additionalPrunableHeaders = Preconditions.checkNotNull(additionalPrunableHeaders);
+    return this;
   }
 
   public boolean shouldCompileHeaders() {
-- 
cgit v1.2.3