1 files changed, 52 insertions, 92 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfTargetsUnderDirectoryFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfTargetsUnderDirectoryFunction.java
index ea210047df..e21ce8b2d2 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfTargetsUnderDirectoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfTargetsUnderDirectoryFunction.java
@@ -13,11 +13,9 @@
 // limitations under the License.
 package com.google.devtools.build.lib.skyframe;
 
-import com.google.common.collect.ImmutableCollection;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
-import com.google.devtools.build.lib.analysis.BlazeDirectories;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier.RepositoryName;
 import com.google.devtools.build.lib.cmdline.ResolvedTargets;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
@@ -27,7 +25,6 @@ import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.pkgcache.FilteringPolicy;
 import com.google.devtools.build.lib.pkgcache.TargetPatternResolverUtil;
 import com.google.devtools.build.lib.skyframe.PrepareDepsOfTargetsUnderDirectoryValue.PrepareDepsOfTargetsUnderDirectoryKey;
-import com.google.devtools.build.lib.skyframe.RecursivePkgValue.RecursivePkgKey;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.RootedPath;
@@ -35,6 +32,8 @@ import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
 
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Map;
 
 import javax.annotation.Nullable;
@@ -43,113 +42,74 @@ import javax.annotation.Nullable;
  * Ensures the graph contains the targets in the directory's package, if any, and in the
  * non-excluded packages in its subdirectories, and all those targets' transitive dependencies,
  * after a successful evaluation.
- *
- * <p>Computes {@link PrepareDepsOfTargetsUnderDirectoryValue} which describes whether the
- * directory is a package and how many non-excluded packages exist below each of the directory's
- * subdirectories.
  */
 public class PrepareDepsOfTargetsUnderDirectoryFunction implements SkyFunction {
-  private final BlazeDirectories directories;
-
-  public PrepareDepsOfTargetsUnderDirectoryFunction(BlazeDirectories directories) {
-    this.directories = directories;
-  }
-
   @Override
   public SkyValue compute(SkyKey skyKey, Environment env) {
     PrepareDepsOfTargetsUnderDirectoryKey argument =
         (PrepareDepsOfTargetsUnderDirectoryKey) skyKey.argument();
     FilteringPolicy filteringPolicy = argument.getFilteringPolicy();
-    RecursivePkgKey recursivePkgKey = argument.getRecursivePkgKey();
-    return new MyTraversalFunction(filteringPolicy).visitDirectory(recursivePkgKey, env);
-  }
-
-  private class MyTraversalFunction
-      extends RecursiveDirectoryTraversalFunction<MyVisitor,
-      PrepareDepsOfTargetsUnderDirectoryValue> {
-
-    private final FilteringPolicy filteringPolicy;
-
-    private MyTraversalFunction(FilteringPolicy filteringPolicy) {
-      super(directories);
-      this.filteringPolicy = filteringPolicy;
-    }
-
-    @Override
-    protected PrepareDepsOfTargetsUnderDirectoryValue getEmptyReturn() {
-      return PrepareDepsOfTargetsUnderDirectoryValue.EMPTY;
-    }
-
-    @Override
-    protected MyVisitor getInitialVisitor() {
-      return new MyVisitor(filteringPolicy);
+    CollectPackagesUnderDirectoryValue collectPackagesUnderDirectoryValue =
+        (CollectPackagesUnderDirectoryValue)
+            env.getValue(CollectPackagesUnderDirectoryValue.key(argument.getRecursivePkgKey()));
+    if (env.valuesMissing()) {
+      return null;
     }
-
-    @Override
-    protected SkyKey getSkyKeyForSubdirectory(RepositoryName repository, RootedPath subdirectory,
-        ImmutableSet<PathFragment> excludedSubdirectoriesBeneathSubdirectory) {
-      return PrepareDepsOfTargetsUnderDirectoryValue.key(repository, subdirectory,
-          excludedSubdirectoriesBeneathSubdirectory, filteringPolicy);
-    }
-
-    @Override
-    protected PrepareDepsOfTargetsUnderDirectoryValue aggregateWithSubdirectorySkyValues(
-        MyVisitor visitor, Map<SkyKey, SkyValue> subdirectorySkyValues) {
-      // Aggregate the child subdirectory package state.
-      ImmutableMap.Builder<RootedPath, Boolean> builder = ImmutableMap.builder();
-      for (SkyKey key : subdirectorySkyValues.keySet()) {
-        PrepareDepsOfTargetsUnderDirectoryKey prepDepsKey =
-            (PrepareDepsOfTargetsUnderDirectoryKey) key.argument();
-        PrepareDepsOfTargetsUnderDirectoryValue prepDepsValue =
-            (PrepareDepsOfTargetsUnderDirectoryValue) subdirectorySkyValues.get(key);
-        boolean packagesInSubdirectory = prepDepsValue.isDirectoryPackage();
-        // If the subdirectory isn't a package, check to see if any of its subdirectories
-        // transitively contain packages.
-        if (!packagesInSubdirectory) {
-          ImmutableCollection<Boolean> subdirectoryValues =
-              prepDepsValue.getSubdirectoryTransitivelyContainsPackages().values();
-          for (Boolean pkgsInSubSub : subdirectoryValues) {
-            if (pkgsInSubSub) {
-              packagesInSubdirectory = true;
-              break;
-            }
-          }
-        }
-        builder.put(prepDepsKey.getRecursivePkgKey().getRootedPath(), packagesInSubdirectory);
+    Map<RootedPath, Boolean> subdirMap =
+        collectPackagesUnderDirectoryValue.getSubdirectoryTransitivelyContainsPackages();
+    List<SkyKey> subdirKeys = new ArrayList<>(subdirMap.size());
+    RepositoryName repositoryName = argument.getRecursivePkgKey().getRepository();
+    ImmutableSet<PathFragment> excludedPaths = argument.getRecursivePkgKey().getExcludedPaths();
+
+    PathFragment baseDir = argument.getRecursivePkgKey().getRootedPath().getRelativePath();
+    for (Map.Entry<RootedPath, Boolean> subdirEntry : subdirMap.entrySet()) {
+      if (subdirEntry.getValue()) {
+        // Keep in rough sync with the logic in RecursiveDirectoryTraversalFunction#visitDirectory.
+        RootedPath subdir = subdirEntry.getKey();
+        PathFragment subdirRelativePath = subdir.getRelativePath().relativeTo(baseDir);
+        ImmutableSet<PathFragment> excludedSubdirectoriesBeneathThisSubdirectory =
+            PathFragment.filterPathsStartingWith(excludedPaths, subdirRelativePath);
+
+        subdirKeys.add(
+            PrepareDepsOfTargetsUnderDirectoryValue.key(
+                repositoryName,
+                subdir,
+                excludedSubdirectoriesBeneathThisSubdirectory,
+                filteringPolicy));
       }
-      return PrepareDepsOfTargetsUnderDirectoryValue.of(visitor.isDirectoryPackage(),
-          builder.build());
     }
-  }
-
-  private static class MyVisitor implements RecursiveDirectoryTraversalFunction.Visitor {
-
-    private final FilteringPolicy filteringPolicy;
-    private boolean isDirectoryPackage;
-
-    private MyVisitor(FilteringPolicy filteringPolicy) {
-      this.filteringPolicy = Preconditions.checkNotNull(filteringPolicy);
-    }
-
-    @Override
-    public void visitPackageValue(Package pkg, Environment env) {
-      isDirectoryPackage = true;
-      loadTransitiveTargets(env, pkg, filteringPolicy);
-    }
-
-    public boolean isDirectoryPackage() {
-      return isDirectoryPackage;
+    if (collectPackagesUnderDirectoryValue.isDirectoryPackage()) {
+      PackageIdentifier packageIdentifier =
+          PackageIdentifier.create(
+              argument.getRecursivePkgKey().getRepository(),
+              argument.getRecursivePkgKey().getRootedPath().getRelativePath());
+      PackageValue pkgValue =
+          (PackageValue)
+              Preconditions.checkNotNull(
+                  env.getValue(PackageValue.key(packageIdentifier)),
+                  collectPackagesUnderDirectoryValue);
+      loadTransitiveTargets(env, pkgValue.getPackage(), filteringPolicy, subdirKeys);
+    } else {
+      env.getValues(subdirKeys);
     }
+    return env.valuesMissing() ? null : PrepareDepsOfTargetsUnderDirectoryValue.INSTANCE;
   }
 
-  private static void loadTransitiveTargets(Environment env, Package pkg,
-      FilteringPolicy filteringPolicy) {
+  // The additionalKeysToRequest argument allows us to batch skyframe dependencies a little more
+  // aggressively. Since the keys computed in this method are independent from any other keys, we
+  // can request our keys together with any other keys that are needed, possibly avoiding a restart.
+  private static void loadTransitiveTargets(
+      Environment env,
+      Package pkg,
+      FilteringPolicy filteringPolicy,
+      Iterable<SkyKey> additionalKeysToRequest) {
     ResolvedTargets<Target> packageTargets =
         TargetPatternResolverUtil.resolvePackageTargets(pkg, filteringPolicy);
     ImmutableList.Builder<SkyKey> builder = ImmutableList.builder();
     for (Target target : packageTargets.getTargets()) {
       builder.add(TransitiveTraversalValue.key(target.getLabel()));
     }
+    builder.addAll(additionalKeysToRequest);
     ImmutableList<SkyKey> skyKeys = builder.build();
     env.getValuesOrThrow(skyKeys, NoSuchPackageException.class, NoSuchTargetException.class);
   }