Defer creating the full `PathFragment` for each dirent until we actually need it.
This is a further optimization (after https://github.com/bazelbuild/bazel/commit/d7a9bb5b1e345097820a89fcda106dc7eb7d3fa0) for the same situation of a recursive glob that considers many dirents but few of which actually belong in the result.
PiperOrigin-RevId: 625123534
Change-Id: I4d2334854dca1bb7b87c28adcdcabf65f0c5f85c
diff --git a/src/main/java/com/google/devtools/build/lib/packages/producers/FragmentProducer.java b/src/main/java/com/google/devtools/build/lib/packages/producers/FragmentProducer.java
index 5618b34..acb433a 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/producers/FragmentProducer.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/producers/FragmentProducer.java
@@ -149,22 +149,14 @@
globDetail, base, fragmentIndex, resultSink, visitedGlobSubTasks);
}
- /**
- * Attempts to add {@linkplain PathFragment the input file path} to {@linkplain ResultSink the
- * result sink} if this is the last fragment in the glob expression and {@linkplain Operation glob
- * operation} is {@link Operation#FILES} or {@link Operation#FILES_AND_DIRS}.
- */
- static void maybeAddFileMatchingToResult(
- PathFragment pathFragment,
- int fragmentIndex,
- GlobDetail globDetail,
- FragmentProducer.ResultSink resultSink) {
+ /** Returns if a matching path at the given pattern index should be added to the result. */
+ static boolean shouldAddFileMatchingToResult(int fragmentIndex, GlobDetail globDetail) {
if (globDetail.globOperation().equals(Operation.SUBPACKAGES)) {
- return;
+ return false;
}
if (fragmentIndex < globDetail.patternFragments().size() - 1) {
- return;
+ return false;
}
- resultSink.acceptPathFragmentWithPackageFragment(pathFragment);
+ return true;
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/producers/PatternWithWildcardProducer.java b/src/main/java/com/google/devtools/build/lib/packages/producers/PatternWithWildcardProducer.java
index 2a8b283..d3594cb 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/producers/PatternWithWildcardProducer.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/producers/PatternWithWildcardProducer.java
@@ -105,20 +105,36 @@
if (dirent.getType() == Dirent.Type.UNKNOWN) {
continue;
}
- if (!UnixGlob.matches(patternFragment, dirent.getName(), globDetail.regexPatternCache())) {
+
+ String direntName = dirent.getName();
+
+ if (!UnixGlob.matches(patternFragment, direntName, globDetail.regexPatternCache())) {
continue;
}
- // At this point, we know that the dirent matches current pattern fragment.
- PathFragment child = base.getChild(dirent.getName());
+ // At this point, we know that the dirent matches current pattern fragment but we don't yet
+ // know if it belongs in the result. Delay creating the full PathFragment until we actually
+ // need it.
+
if (dirent.getType() == Dirent.Type.SYMLINK) {
tasks.enqueue(
new SymlinkProducer(
- FileValue.key(RootedPath.toRootedPath(globDetail.packageRoot(), child)),
+ FileValue.key(
+ RootedPath.toRootedPath(globDetail.packageRoot(), base.getChild(direntName))),
(SymlinkProducer.ResultSink) this));
++symlinksCount;
+ } else if (dirent.getType() == Dirent.Type.DIRECTORY) {
+ tasks.enqueue(
+ new DirectoryDirentProducer(
+ globDetail,
+ base.getChild(direntName),
+ fragmentIndex,
+ resultSink,
+ visitedGlobSubTasks));
} else {
- handleDirent(child, dirent.getType() == Dirent.Type.DIRECTORY, tasks);
+ if (FragmentProducer.shouldAddFileMatchingToResult(fragmentIndex, globDetail)) {
+ resultSink.acceptPathFragmentWithPackageFragment(base.getChild(direntName));
+ }
}
}
@@ -179,9 +195,17 @@
return DONE;
}
- // When creating `DirentProducer` for symlinks, pass in the symlink path instead of the target
- // path.
- handleDirent(symlinkKey.argument().getRootRelativePath(), symlinkValue.isDirectory(), tasks);
+ // Use the symlink path instead of the target path.
+ PathFragment direntPath = symlinkKey.argument().getRootRelativePath();
+ if (symlinkValue.isDirectory()) {
+ tasks.enqueue(
+ new DirectoryDirentProducer(
+ globDetail, direntPath, fragmentIndex, resultSink, visitedGlobSubTasks));
+ } else {
+ if (FragmentProducer.shouldAddFileMatchingToResult(fragmentIndex, globDetail)) {
+ resultSink.acceptPathFragmentWithPackageFragment(direntPath);
+ }
+ }
}
// After all symlinks of dirents are processed, `symlinks` array list is useless and should be
@@ -189,15 +213,4 @@
symlinks = null;
return DONE;
}
-
- private void handleDirent(PathFragment pathFragment, boolean isDir, Tasks tasks) {
- if (isDir) {
- tasks.enqueue(
- new DirectoryDirentProducer(
- globDetail, pathFragment, fragmentIndex, resultSink, visitedGlobSubTasks));
- } else {
- FragmentProducer.maybeAddFileMatchingToResult(
- pathFragment, fragmentIndex, globDetail, resultSink);
- }
- }
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/producers/PatternWithoutWildcardProducer.java b/src/main/java/com/google/devtools/build/lib/packages/producers/PatternWithoutWildcardProducer.java
index 4cf8a03..6a18f4a 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/producers/PatternWithoutWildcardProducer.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/producers/PatternWithoutWildcardProducer.java
@@ -91,7 +91,9 @@
return new DirectoryDirentProducer(
globDetail, filePath, fragmentIndex, resultSink, visitedGlobSubTasks);
}
- FragmentProducer.maybeAddFileMatchingToResult(filePath, fragmentIndex, globDetail, resultSink);
+ if (FragmentProducer.shouldAddFileMatchingToResult(fragmentIndex, globDetail)) {
+ resultSink.acceptPathFragmentWithPackageFragment(filePath);
+ }
return DONE;
}
}
