Add --experimental_check_external_files

This flag mirrors `--experimental_check_output_files` to allow installations with a large number of external files to not scan them for changes on every bazel invocation. If used in combination with `--experimental_check_output_files` and `--watchfs` you can now avoid scanning the filesystem entirely allowing large projects to keep many things cached while still executing small `bazel run` commands quickly. Fixes bazelbuild#14400
aspect-build · Dec 14, 2021 · d1dd5cf · d1dd5cf
1 parent 5aef53a
commit d1dd5cf
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 15 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/PackageOptions.java b/src/main/java/com/google/devtools/build/lib/pkgcache/PackageOptions.java
@@ -189,10 +189,23 @@ public ParallelismConverter() throws OptionsParsingException {
     effectTags = {OptionEffectTag.UNKNOWN},
     help =
         "Check for modifications made to the output files of a build. Consider setting "
-            + "this flag to false to see the effect on incremental build times."
+            + "this flag to false if you don't expect these files to change outside of bazel "
+            + "since it will speed up incremental build times."
   )
   public boolean checkOutputFiles;
 
+  @Option(
+    name = "experimental_check_external_files",
+    defaultValue = "true",
+    documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+    effectTags = {OptionEffectTag.UNKNOWN},
+    help =
+        "Check for modifications made to the external files of a build. Consider setting "
+            + "this flag to false if you don't expect these files to change outside of bazel "
+            + "since it will speed up incremental build times."
+  )
+  public boolean checkExternalFiles;
+
   /**
    * A converter from strings containing comma-separated names of packages to lists of strings.
    */

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
@@ -273,7 +273,7 @@ public WorkspaceInfoFromDiff sync(
         options);
     long startTime = System.nanoTime();
     WorkspaceInfoFromDiff workspaceInfo =
-        handleDiffs(eventHandler, packageOptions.checkOutputFiles, options);
+        handleDiffs(eventHandler, packageOptions.checkOutputFiles, packageOptions.checkExternalFiles, options);
     long stopTime = System.nanoTime();
     Profiler.instance().logSimpleTask(startTime, stopTime, ProfilerTask.INFO, "handleDiffs");
     long duration = stopTime - startTime;
@@ -333,12 +333,12 @@ public void handleDiffsForTesting(ExtendedEventHandler eventHandler)
       dropConfiguredTargetsNow(eventHandler);
       super.lastAnalysisDiscarded = false;
     }
-    handleDiffs(eventHandler, /*checkOutputFiles=*/ false, OptionsProvider.EMPTY);
+    handleDiffs(eventHandler, /*checkOutputFiles=*/ false, /*checkExternalFiles=*/ true, OptionsProvider.EMPTY);
   }
 
   @Nullable
   private WorkspaceInfoFromDiff handleDiffs(
-      ExtendedEventHandler eventHandler, boolean checkOutputFiles, OptionsProvider options)
+      ExtendedEventHandler eventHandler, boolean checkOutputFiles, boolean checkExternalFiles, OptionsProvider options)
       throws InterruptedException, AbruptExitException {
     TimestampGranularityMonitor tsgm = this.tsgm.get();
     modifiedFiles = 0;
@@ -376,13 +376,18 @@ private WorkspaceInfoFromDiff handleDiffs(
     int fsvcThreads = buildRequestOptions == null ? 200 : buildRequestOptions.fsvcThreads;
     handleDiffsWithCompleteDiffInformation(
         tsgm, modifiedFilesByPathEntry, managedDirectoriesChanged, fsvcThreads);
-    handleDiffsWithMissingDiffInformation(
-        eventHandler,
-        tsgm,
-        pathEntriesWithoutDiffInformation,
-        checkOutputFiles,
-        managedDirectoriesChanged,
-        fsvcThreads);
+    if (checkOutputFiles || checkExternalFiles || !pathEntriesWithoutDiffInformation.isEmpty()) {
+      handleDiffsWithMissingDiffInformation(
+          eventHandler,
+          tsgm,
+          pathEntriesWithoutDiffInformation,
+          checkOutputFiles,
+          checkExternalFiles,
+          managedDirectoriesChanged,
+          fsvcThreads);
+    } else {
+      logger.atInfo().log("Skipping scanning the filesystem for cache invalidations, yay!");
+    }
     handleClientEnvironmentChanges();
     return workspaceInfo;
   }
@@ -472,6 +477,7 @@ private void handleDiffsWithMissingDiffInformation(
       TimestampGranularityMonitor tsgm,
       Set<Pair<Root, ProcessableModifiedFileSet>> pathEntriesWithoutDiffInformation,
       boolean checkOutputFiles,
+      boolean checkExternalFiles,
       boolean managedDirectoriesChanged,
       int fsvcThreads)
       throws InterruptedException {
@@ -513,14 +519,20 @@ private void handleDiffsWithMissingDiffInformation(
         diffPackageRootsUnderWhichToCheck.add(pair.getFirst());
       }
 
-      EnumSet<FileType> fileTypesToCheck =
-          EnumSet.of(FileType.EXTERNAL_REPO, FileType.EXTERNAL_IN_MANAGED_DIRECTORY);
+      EnumSet<FileType> fileTypesToCheck = EnumSet.noneOf(FileType.class);
+      if (checkExternalFiles) {
+        fileTypesToCheck =
+            EnumSet.of(FileType.EXTERNAL_REPO, FileType.EXTERNAL_IN_MANAGED_DIRECTORY);
+        if (externalFilesKnowledge.tooManyNonOutputExternalFilesSeen) {
+          fileTypesToCheck.add(FileType.EXTERNAL);
+        }
+      }
       // See the comment for FileType.OUTPUT for why we need to consider output files here.
       if (checkOutputFiles) {
         fileTypesToCheck.add(FileType.OUTPUT);
       }
-      if (externalFilesKnowledge.tooManyNonOutputExternalFilesSeen) {
-        fileTypesToCheck.add(FileType.EXTERNAL);
+      if (!checkExternalFiles && !checkOutputFiles && diffPackageRootsUnderWhichToCheck.isEmpty()) {
+        throw new AssertionError("should have already early exited");
       }
       logger.atInfo().log(
           "About to scan skyframe graph checking for filesystem nodes of types %s",