diff options
Diffstat (limited to 'src')
5 files changed, 33 insertions, 4 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java index f8ad17c053..8fa921918b 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java @@ -1586,11 +1586,17 @@ public final class PackageFactory { boolean wasPreprocessed, Path buildFilePath, Globber globber, RuleVisibility defaultVisibility, MakeEnvironment.Builder pkgMakeEnv) throws InterruptedException { - if (wasPreprocessed) { - // No point in prefetching globs here: preprocessing implies eager evaluation - // of all globs. + if (wasPreprocessed && preprocessorFactory.considersGlobs()) { + // All the globs have either already been evaluated and they aren't in the ast anymore, or + // they are in the ast but the globber has been evaluating them lazily and so there is no + // point in prefetching them again. return; } + // TODO(bazel-team): It may be wasteful to evaluate the BUILD file here, only to throw away the + // result. It may be better to first scan the ast and see if there are even possibly any globs + // at all. Additionally, it's wasteful to execute Skylark code that cannot invoke globs. So one + // strategy would be to crawl the ast and tag statements whose execution cannot involve globs - + // these can be executed and their impact on the resulting package can be saved. try (Mutability mutability = Mutability.create("prefetchGlobs for %s", packageId)) { Environment pkgEnv = Environment.builder(mutability) .setGlobals(Environment.BUILD) diff --git a/src/main/java/com/google/devtools/build/lib/packages/Preprocessor.java b/src/main/java/com/google/devtools/build/lib/packages/Preprocessor.java index 53e383454c..1bf5654348 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Preprocessor.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Preprocessor.java @@ -68,6 +68,14 @@ public interface Preprocessor { boolean isStillValid(); /** + * Returns whether all globs encountered during {@link Preprocessor#preprocess} will be passed + * along to the {@link Globber} given there (which then executes them asynchronously). If this + * is not the case, then e.g. prefetching globs during normal BUILD file evaluation may be + * profitable. + */ + boolean considersGlobs(); + + /** * Returns a Preprocessor instance capable of preprocessing a BUILD file independently (e.g. it * ought to be fine to call {@link #getPreprocessor} for each BUILD file). */ @@ -87,6 +95,11 @@ public interface Preprocessor { } @Override + public boolean considersGlobs() { + return false; + } + + @Override public Preprocessor getPreprocessor() { return null; } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java index 0a69fdd50a..4d8596f27a 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java @@ -894,7 +894,7 @@ public class PackageFunction implements SkyFunction { // If the globber was used to evaluate globs during preprocessing, it's important that we // reuse that globber during BUILD file evaluation for two reasons: (i) correctness, since // Skyframe deps are added after the fact (ii) performance, in the case that globs were - // fetched lazily during preprocessing. + // fetched lazily during preprocessing. See Preprocessor.Factory#considersGlobs. Globber globber = astAfterPreprocessing.globber != null ? astAfterPreprocessing.globber : packageFactory.createLegacyGlobber(buildFilePath.getParentDirectory(), diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/PreprocessorUtils.java b/src/test/java/com/google/devtools/build/lib/packages/util/PreprocessorUtils.java index b4f674b41c..7bfb31ec2d 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/PreprocessorUtils.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/PreprocessorUtils.java @@ -53,6 +53,11 @@ public class PreprocessorUtils { } @Override + public boolean considersGlobs() { + return false; + } + + @Override @Nullable public Preprocessor getPreprocessor() { return preprocessor; diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/SubincludePreprocessor.java b/src/test/java/com/google/devtools/build/lib/packages/util/SubincludePreprocessor.java index b192237d99..74e615f812 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/SubincludePreprocessor.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/SubincludePreprocessor.java @@ -59,6 +59,11 @@ public class SubincludePreprocessor implements Preprocessor { } @Override + public boolean considersGlobs() { + return false; + } + + @Override public Preprocessor getPreprocessor() { return new SubincludePreprocessor(fileSystem, loc); } |