Follow-up on f9fdc8dfced8b2b14561720623126a91e04b22cb -- reinstate short-circuit check when all package paths have good diff information and no external files have been seen.
--
MOS_MIGRATED_REVID=109703164
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java b/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java
index 100858a..f21e4ca 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java
@@ -27,6 +27,10 @@
private final AtomicReference<PathPackageLocator> pkgLocator;
private final ExternalFileAction externalFileAction;
+ // This variable is set to true from multiple threads, but only read once, in the main thread.
+ // So volatility or an AtomicBoolean is not needed.
+ private boolean externalFileSeen = false;
+
/**
* @param pkgLocator an {@link AtomicReference} to a {@link PathPackageLocator} used to
* determine what files are internal.
@@ -50,15 +54,11 @@
ERROR_OUT,
}
- private enum FileType {
- // A file inside the package roots or in an external repository.
- INTERNAL_FILE,
-
- // A file outside the package roots about which we may make no other assumptions.
- EXTERNAL_MUTABLE_FILE,
+ boolean isExternalFileSeen() {
+ return externalFileSeen;
}
- public static boolean isInternal(RootedPath rootedPath, PathPackageLocator packageLocator) {
+ static boolean isInternal(RootedPath rootedPath, PathPackageLocator packageLocator) {
// TODO(bazel-team): This is inefficient when there are a lot of package roots or there are a
// lot of external directories. Consider either explicitly preventing this case or using a more
// efficient approach here (e.g. use a trie for determining if a file is under an external
@@ -77,6 +77,7 @@
return;
}
+ externalFileSeen = true;
if (externalFileAction == ExternalFileAction.DEPEND_ON_EXTERNAL_PKG) {
// For files outside the package roots, add a dependency on the //external package so that if
// the WORKSPACE file changes, the File/DirectoryStateValue will be re-evaluated.
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 b7f3757..0519d76 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
@@ -326,6 +326,14 @@
private void handleDiffsWithMissingDiffInformation(EventHandler eventHandler,
Set<Pair<Path, DiffAwarenessManager.ProcessableModifiedFileSet>>
pathEntriesWithoutDiffInformation) throws InterruptedException {
+ if (pathEntriesWithoutDiffInformation.isEmpty()
+ && Iterables.isEmpty(customDirtinessCheckers)
+ && !externalFilesHelper.isExternalFileSeen()) {
+ // Avoid a full graph scan if we have good diff information for all path entries, there are
+ // no custom checkers that need to look at the whole graph, and no external (not under any
+ // path) files need to be checked.
+ return;
+ }
// Before running the FilesystemValueChecker, ensure that all values marked for invalidation
// have actually been invalidated (recall that invalidation happens at the beginning of the
// next evaluate() call), because checking those is a waste of time.
