From 833fd12fd86ddc32e89b1b0cff57b248a58dc8ab Mon Sep 17 00:00:00 2001
From: laurentlb <laurentlb@google.com>
Date: Thu, 30 Mar 2017 20:15:08 +0000
Subject: Improve glob prefetching performance by providing a more realistic
 environment.

That means the prefetching execution is more similar to the real execution,
it will fetch more globs and throw less exceptions.

In the code path, one exception was thrown for almost each statement. With
this change, many BUILD files can be evaluated without an exception.

RELNOTES: None.

PiperOrigin-RevId: 151740684
---
 .../build/lib/packages/PackageFactory.java         | 67 ++++++++++++++++------
 1 file changed, 51 insertions(+), 16 deletions(-)

(limited to 'src/main/java/com')

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 16df4e0b51..35c0cfb241 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
@@ -1341,8 +1341,15 @@ public final class PackageFactory {
     try {
       // At this point the package is guaranteed to exist.  It may have parse or
       // evaluation errors, resulting in a diminished number of rules.
-      prefetchGlobs(packageId, astAfterPreprocessing.ast, astAfterPreprocessing.preprocessed,
-          buildFile, globber, defaultVisibility, makeEnv);
+      prefetchGlobs(
+          packageId,
+          astAfterPreprocessing.ast,
+          astAfterPreprocessing.preprocessed,
+          buildFile,
+          globber,
+          defaultVisibility,
+          makeEnv,
+          imports);
       return evaluateBuildFile(
           workspaceName,
           packageId,
@@ -1607,7 +1614,23 @@ public final class PackageFactory {
         builder.build(), "no native function or rule '%s'");
   }
 
-  private void buildPkgEnv(Environment pkgEnv, PackageContext context, RuleFactory ruleFactory) {
+  /** A function that does nothing and ignores the arguments. */
+  private final BaseFunction noopFunction =
+      new BaseFunction("noop", FunctionSignature.KWARGS) {
+        @Override
+        public Object call(Object[] arguments, FuncallExpression ast, Environment env)
+            throws EvalException {
+          return Runtime.NONE;
+        }
+      };
+
+  /** @param fakeEnv specify if we declare no-op functions, or real functions. */
+  private void buildPkgEnv(
+      Environment pkgEnv,
+      PackageContext context,
+      RuleFactory ruleFactory,
+      PackageIdentifier packageId,
+      boolean fakeEnv) {
     // TODO(bazel-team): remove the naked functions that are redundant with the nativeModule,
     // or if not possible, at least make them straight copies from the native module variant.
     // or better, use a common Environment.Frame for these common bindings
@@ -1625,12 +1648,20 @@ public final class PackageFactory {
 
     for (String ruleClass : ruleFactory.getRuleClassNames()) {
       BaseFunction ruleFunction = newRuleFunction(ruleFactory, ruleClass);
-      pkgEnv.setup(ruleClass, ruleFunction);
+      if (fakeEnv) {
+        pkgEnv.setup(ruleClass, ruleFunction);
+      } else {
+        pkgEnv.setup(ruleClass, noopFunction);
+      }
     }
 
     for (EnvironmentExtension extension : environmentExtensions) {
       extension.update(pkgEnv);
     }
+
+    pkgEnv.setupDynamic(PKG_CONTEXT, context);
+    pkgEnv.setupDynamic(Runtime.PKG_NAME, packageId.getPackageFragment().getPathString());
+    pkgEnv.setupDynamic(Runtime.REPOSITORY_NAME, packageId.getRepository().toString());
   }
 
   /**
@@ -1702,10 +1733,7 @@ public final class PackageFactory {
       PackageContext context =
           new PackageContext(
               pkgBuilder, globber, eventHandler, ruleFactory.getAttributeContainerFactory());
-      buildPkgEnv(pkgEnv, context, ruleFactory);
-      pkgEnv.setupDynamic(PKG_CONTEXT, context);
-      pkgEnv.setupDynamic(Runtime.PKG_NAME, packageId.getPackageFragment().getPathString());
-      pkgEnv.setupDynamic(Runtime.REPOSITORY_NAME, packageId.getRepository().toString());
+      buildPkgEnv(pkgEnv, context, ruleFactory, packageId, true);
 
       if (containsError) {
         pkgBuilder.setContainsErrors();
@@ -1736,12 +1764,16 @@ public final class PackageFactory {
     return pkgBuilder;
   }
 
-  /**
-   * Visit all targets and expand the globs in parallel.
-   */
-  private void prefetchGlobs(PackageIdentifier packageId, BuildFileAST buildFileAST,
-      boolean wasPreprocessed, Path buildFilePath, Globber globber,
-      RuleVisibility defaultVisibility, MakeEnvironment.Builder pkgMakeEnv)
+  /** Visit all targets and expand the globs in parallel. */
+  private void prefetchGlobs(
+      PackageIdentifier packageId,
+      BuildFileAST buildFileAST,
+      boolean wasPreprocessed,
+      Path buildFilePath,
+      Globber globber,
+      RuleVisibility defaultVisibility,
+      MakeEnvironment.Builder pkgMakeEnv,
+      Map<String, Extension> imports)
       throws InterruptedException {
     if (wasPreprocessed && preprocessorFactory.considersGlobs()) {
       // All the globs have either already been evaluated and they aren't in the ast anymore, or
@@ -1759,6 +1791,7 @@ public final class PackageFactory {
           Environment.builder(mutability)
               .setGlobals(BazelLibrary.GLOBALS)
               .setEventHandler(NullEventHandler.INSTANCE)
+              .setImportedExtensions(imports)
               .setPhase(Phase.LOADING)
               .build();
       SkylarkUtils.setToolsRepository(pkgEnv, ruleClassProvider.getToolsRepository());
@@ -1780,12 +1813,14 @@ public final class PackageFactory {
               globber,
               NullEventHandler.INSTANCE,
               ruleFactory.getAttributeContainerFactory());
-      buildPkgEnv(pkgEnv, context, ruleFactory);
+      buildPkgEnv(pkgEnv, context, ruleFactory, packageId, false);
+
       try {
         pkgEnv.update("glob", newGlobFunction.apply(context, /*async=*/true));
         // The Fileset function is heavyweight in that it can run glob(). Avoid this during the
         // preloading phase.
-        pkgEnv.update("FilesetEntry", Runtime.NONE);
+        pkgEnv.update("FilesetEntry", noopFunction);
+        pkgEnv.update("vardef", noopFunction);
       } catch (EvalException e) {
         throw new AssertionError(e);
       }
-- 
cgit v1.2.3