From f1e257da1467760e7d2f4b3b1f651484f4fe7b67 Mon Sep 17 00:00:00 2001 From: Janak Ramakrishnan Date: Thu, 7 Jan 2016 17:14:59 +0000 Subject: Split PrepareDepsOfTargetsUnderDirectory into two parts, one which does the directory traversal and package loading, and the other which requests deps on all the transitive targets. We need values from the first half, but the second half can fail to evaluate because of a target cycle. By splitting them, we ensure that there will be values in the graph, so we can get the targets below a directory even if there are cycles present. -- MOS_MIGRATED_REVID=111609889 --- ...PrepareDepsOfTargetsUnderDirectoryFunction.java | 144 ++++++++------------- 1 file changed, 52 insertions(+), 92 deletions(-) (limited to 'src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfTargetsUnderDirectoryFunction.java') 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. - * - *

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 { - - 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 excludedSubdirectoriesBeneathSubdirectory) { - return PrepareDepsOfTargetsUnderDirectoryValue.key(repository, subdirectory, - excludedSubdirectoriesBeneathSubdirectory, filteringPolicy); - } - - @Override - protected PrepareDepsOfTargetsUnderDirectoryValue aggregateWithSubdirectorySkyValues( - MyVisitor visitor, Map subdirectorySkyValues) { - // Aggregate the child subdirectory package state. - ImmutableMap.Builder 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 subdirectoryValues = - prepDepsValue.getSubdirectoryTransitivelyContainsPackages().values(); - for (Boolean pkgsInSubSub : subdirectoryValues) { - if (pkgsInSubSub) { - packagesInSubdirectory = true; - break; - } - } - } - builder.put(prepDepsKey.getRecursivePkgKey().getRootedPath(), packagesInSubdirectory); + Map subdirMap = + collectPackagesUnderDirectoryValue.getSubdirectoryTransitivelyContainsPackages(); + List subdirKeys = new ArrayList<>(subdirMap.size()); + RepositoryName repositoryName = argument.getRecursivePkgKey().getRepository(); + ImmutableSet excludedPaths = argument.getRecursivePkgKey().getExcludedPaths(); + + PathFragment baseDir = argument.getRecursivePkgKey().getRootedPath().getRelativePath(); + for (Map.Entry 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 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 additionalKeysToRequest) { ResolvedTargets packageTargets = TargetPatternResolverUtil.resolvePackageTargets(pkg, filteringPolicy); ImmutableList.Builder builder = ImmutableList.builder(); for (Target target : packageTargets.getTargets()) { builder.add(TransitiveTraversalValue.key(target.getLabel())); } + builder.addAll(additionalKeysToRequest); ImmutableList skyKeys = builder.build(); env.getValuesOrThrow(skyKeys, NoSuchPackageException.class, NoSuchTargetException.class); } -- cgit v1.2.3