From 9c9b85f319520f91d48389fc1408c8710af62c29 Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 31 Jul 2018 06:57:06 -0700 Subject: Slightly improve UnixGlob implementation. For recursive patterns, the current implementation leads to two identical recursive calls. This isn't harmful as there is a cache that will make the second call very cheap, but does not seem right. As a consequence, a cache is only needed if there are two recursive patterns in a single glob-path, e.g. /a/b/**/c/**/d/... as either "**" can expand to 0 or more directories (so /a/b/c/c/d could be reached in two different ways). If there is only a single recursive pattern, even with "*" placeholders, there is always a unique expansion (a "*" always represents exactly one path element). RELNOTES: None. PiperOrigin-RevId: 206753919 --- .../google/devtools/build/lib/vfs/UnixGlob.java | 23 ++++++++++------------ 1 file changed, 10 insertions(+), 13 deletions(-) (limited to 'src/main/java/com/google') diff --git a/src/main/java/com/google/devtools/build/lib/vfs/UnixGlob.java b/src/main/java/com/google/devtools/build/lib/vfs/UnixGlob.java index 24cdeaeb3e..ab421f218c 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/UnixGlob.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/UnixGlob.java @@ -568,14 +568,13 @@ public final class UnixGlob { pendingOps.incrementAndGet(); try { for (String[] splitPattern : splitPatterns) { - boolean containsRecursivePattern = false; + int numRecursivePatterns = 0; for (String pattern : splitPattern) { if (isRecursivePattern(pattern)) { - containsRecursivePattern = true; - break; + ++numRecursivePatterns; } } - GlobTaskContext context = containsRecursivePattern + GlobTaskContext context = numRecursivePatterns > 1 ? new RecursiveGlobTaskContext(splitPattern, excludeDirectories, dirPred, syscalls) : new GlobTaskContext(splitPattern, excludeDirectories, dirPred, syscalls); context.queueGlob(base, baseStat.isDirectory(), 0); @@ -819,20 +818,18 @@ public final class UnixGlob { } boolean childIsDir = (type == Dirent.Type.DIRECTORY); String text = dent.getName(); - // Optimize allocations for the case where the pattern doesn't match the dirent. - Path child = null; if (isRecursivePattern) { - // Recurse without shifting the pattern. + Path child = base.getChild(text); + // Recurse without shifting the pattern. The case where we shifting the pattern is + // already handled by the special case above. if (childIsDir) { - child = base.getChild(text); context.queueGlob(child, childIsDir, idx); + } else if (idx + 1 == context.patternParts.length) { + results.add(child); } - } - if (matches(pattern, text, cache)) { - if (child == null) { - child = base.getChild(text); - } + } else if (matches(pattern, text, cache)) { + Path child = base.getChild(text); // Recurse and consume one segment of the pattern. if (childIsDir) { -- cgit v1.2.3