From d1dd5cfaef157ae773607106c564416d40bed7fe Mon Sep 17 00:00:00 2001 From: Paul Tarjan Date: Fri, 10 Dec 2021 09:20:09 -0800 Subject: [PATCH] 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 #14400 --- .../build/lib/pkgcache/PackageOptions.java | 15 ++++++- .../skyframe/SequencedSkyframeExecutor.java | 40 ++++++++++++------- 2 files changed, 40 insertions(+), 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 index ca6f59321831a1..1cbdc2935d67b8 100644 --- 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 index 3a56b5c73b0f11..cc09a20f71bc79 100644 --- 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> pathEntriesWithoutDiffInformation, boolean checkOutputFiles, + boolean checkExternalFiles, boolean managedDirectoriesChanged, int fsvcThreads) throws InterruptedException { @@ -513,14 +519,20 @@ private void handleDiffsWithMissingDiffInformation( diffPackageRootsUnderWhichToCheck.add(pair.getFirst()); } - EnumSet fileTypesToCheck = - EnumSet.of(FileType.EXTERNAL_REPO, FileType.EXTERNAL_IN_MANAGED_DIRECTORY); + EnumSet 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",