Add bulk package lookup for use during target pattern resolution.
--
MOS_MIGRATED_REVID=111130363
diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/RecursivePackageProvider.java b/src/main/java/com/google/devtools/build/lib/pkgcache/RecursivePackageProvider.java
index 24866b6..808085b 100644
--- a/src/main/java/com/google/devtools/build/lib/pkgcache/RecursivePackageProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/pkgcache/RecursivePackageProvider.java
@@ -14,10 +14,16 @@
package com.google.devtools.build.lib.pkgcache;
import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.PackageIdentifier.RepositoryName;
+import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.packages.NoSuchPackageException;
+import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.RootedPath;
+import java.util.Map;
+
/**
* Support for resolving {@code package/...} target patterns.
*/
@@ -31,4 +37,23 @@
*/
Iterable<PathFragment> getPackagesUnderDirectory(RepositoryName repository,
PathFragment directory, ImmutableSet<PathFragment> excludedSubdirectories);
+
+
+ /**
+ * Returns the {@link Package} corresponding to each Package in "pkgIds". If any of the packages
+ * does not exist (e.g. {@code isPackage(pkgIds)} returns false), throws a
+ * {@link NoSuchPackageException}.
+ *
+ * <p>The returned package may contain lexical/grammatical errors, in which case
+ * <code>pkg.containsErrors() == true</code>. Such packages may be missing some rules. Any rules
+ * that are present may soundly be used for builds, though.
+ *
+ * @param eventHandler the eventHandler on which to report warning and errors; if the package
+ * has been loaded by another thread, this eventHandler won't see any warnings or errors
+ * @param pkgIds an Iterable of PackageIdentifier objects.
+ * @throws NoSuchPackageException if any package could not be found.
+ * @throws InterruptedException if the package loading was interrupted.
+ */
+ Map<PackageIdentifier, Package> bulkGetPackages(EventHandler eventHandler,
+ Iterable<PackageIdentifier> pkgIds) throws NoSuchPackageException, InterruptedException;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java b/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java
index 115a0f5..db6639d 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.cmdline.Label;
@@ -37,6 +38,7 @@
import java.util.ArrayList;
import java.util.List;
+import java.util.Map;
/**
* A {@link RecursivePackageProvider} backed by an {@link Environment}. Its methods
@@ -78,6 +80,17 @@
}
@Override
+ public Map<PackageIdentifier, Package> bulkGetPackages(EventHandler eventHandler,
+ Iterable<PackageIdentifier> pkgIds)
+ throws NoSuchPackageException, InterruptedException {
+ ImmutableMap.Builder<PackageIdentifier, Package> builder = ImmutableMap.builder();
+ for (PackageIdentifier pkgId : pkgIds) {
+ builder.put(pkgId, getPackage(eventHandler, pkgId));
+ }
+ return builder.build();
+ }
+
+ @Override
public boolean isPackage(EventHandler eventHandler, PackageIdentifier packageId)
throws MissingDepException {
SkyKey packageLookupKey = PackageLookupValue.key(packageId);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java b/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java
index 2768c31..f8761ba 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java
@@ -15,11 +15,13 @@
import com.google.common.base.Function;
import com.google.common.base.Objects;
+import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
+import com.google.common.collect.Sets.SetView;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.PackageIdentifier.RepositoryName;
@@ -90,6 +92,36 @@
}
@Override
+ public Map<PackageIdentifier, Package> bulkGetPackages(EventHandler eventHandler,
+ Iterable<PackageIdentifier> pkgIds) throws NoSuchPackageException {
+ Set<SkyKey> pkgKeys = ImmutableSet.copyOf(PackageValue.keys(pkgIds));
+
+ ImmutableMap.Builder<PackageIdentifier, Package> pkgResults = ImmutableMap.builder();
+ Map<SkyKey, SkyValue> packages = graph.getSuccessfulValues(pkgKeys);
+ for (PackageIdentifier pkgId : pkgIds) {
+ PackageValue pkgValue = (PackageValue) packages.get(PackageValue.key(pkgId));
+ pkgResults.put(pkgId, Preconditions.checkNotNull(pkgValue.getPackage(), pkgId));
+ }
+
+ SetView<SkyKey> unknownKeys = Sets.difference(pkgKeys, packages.keySet());
+ for (Map.Entry<SkyKey, Exception> missingOrExceptionEntry :
+ graph.getMissingAndExceptions(unknownKeys).entrySet()) {
+ PackageIdentifier pkgIdentifier =
+ (PackageIdentifier) missingOrExceptionEntry.getKey().argument();
+ Exception exception = missingOrExceptionEntry.getValue();
+ if (exception == null) {
+ // If the package key does not exist in the graph, then it must not correspond to any
+ // package, because the SkyQuery environment has already loaded the universe.
+ throw new BuildFileNotFoundException(pkgIdentifier, "BUILD file not found on package path");
+ }
+ Throwables.propagateIfInstanceOf(exception, NoSuchPackageException.class);
+ Throwables.propagate(exception);
+ }
+ return pkgResults.build();
+ }
+
+
+ @Override
public boolean isPackage(EventHandler eventHandler, PackageIdentifier packageName) {
SkyKey packageLookupKey = PackageLookupValue.key(packageName);
if (!graph.exists(packageLookupKey)) {
@@ -162,21 +194,6 @@
return builder.build();
}
- private final void preloadPackages(final RepositoryName repositoryName,
- Iterable<TraversalInfo> traversals) {
- // Note that the return value is intentionally unused - we call this to elicit the side effect
- // that the packages end up warm in the graph.
- graph.getSuccessfulValues(Iterables.transform(traversals,
- new Function<TraversalInfo, SkyKey>() {
- @Override
- public SkyKey apply(TraversalInfo traversalInfo) {
- return PackageValue.key(
- PackageIdentifier.create(repositoryName,
- traversalInfo.rootedDir.getRelativePath()));
- }
- }));
- }
-
private void collectPackagesUnder(final RepositoryName repository,
Set<TraversalInfo> traversals, ImmutableList.Builder<PathFragment> builder,
final FilteringPolicy policy) {
@@ -189,9 +206,6 @@
}
});
Map<SkyKey, SkyValue> values = graph.getSuccessfulValues(traversalToKeyMap.values());
- // Preload the packages so subsequent lookups are faster. A better approach might be forego this
- // here, and instead load the needed package in batch on demand.
- preloadPackages(repository, traversals);
ImmutableSet.Builder<TraversalInfo> subdirTraversalBuilder = ImmutableSet.builder();
for (Map.Entry<TraversalInfo, SkyKey> entry : traversalToKeyMap.entrySet()) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java
index 3f5f76e..d3f758f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java
@@ -13,7 +13,10 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
+import com.google.common.base.Function;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.PackageIdentifier.RepositoryName;
@@ -32,6 +35,8 @@
import com.google.devtools.build.lib.pkgcache.TargetPatternResolverUtil;
import com.google.devtools.build.lib.vfs.PathFragment;
+import java.util.Map;
+
/**
* A {@link TargetPatternResolver} backed by a {@link RecursivePackageProvider}.
*/
@@ -65,6 +70,11 @@
return recursivePackageProvider.getPackage(eventHandler, pkgIdentifier);
}
+ private Map<PackageIdentifier, Package> bulkGetPackages(Iterable<PackageIdentifier> pkgIds)
+ throws NoSuchPackageException, InterruptedException {
+ return recursivePackageProvider.bulkGetPackages(eventHandler, pkgIds);
+ }
+
@Override
public Target getTargetOrNull(Label label) throws InterruptedException {
try {
@@ -113,6 +123,26 @@
}
}
+ private Map<PackageIdentifier, ResolvedTargets<Target>> bulkGetTargetsInPackage(
+ String originalPattern,
+ Iterable<PackageIdentifier> pkgIds, FilteringPolicy policy)
+ throws TargetParsingException, InterruptedException {
+ try {
+ Map<PackageIdentifier, Package> pkgs = bulkGetPackages(pkgIds);
+ ImmutableMap.Builder<PackageIdentifier, ResolvedTargets<Target>> result =
+ ImmutableMap.builder();
+ for (PackageIdentifier pkgId : pkgIds) {
+ Package pkg = pkgs.get(pkgId);
+ result.put(pkgId, TargetPatternResolverUtil.resolvePackageTargets(pkg, policy));
+ }
+ return result.build();
+ } catch (NoSuchThingException e) {
+ String message = TargetPatternResolverUtil.getParsingErrorMessage(
+ e.getMessage(), originalPattern);
+ throw new TargetParsingException(message, e);
+ }
+ }
+
@Override
public boolean isPackage(PackageIdentifier packageIdentifier) {
return recursivePackageProvider.isPackage(eventHandler, packageIdentifier);
@@ -124,7 +154,7 @@
}
@Override
- public ResolvedTargets<Target> findTargetsBeneathDirectory(RepositoryName repository,
+ public ResolvedTargets<Target> findTargetsBeneathDirectory(final RepositoryName repository,
String originalPattern, String directory, boolean rulesOnly,
ImmutableSet<String> excludedSubdirectories)
throws TargetParsingException, InterruptedException {
@@ -138,10 +168,17 @@
Iterable<PathFragment> packagesUnderDirectory =
recursivePackageProvider.getPackagesUnderDirectory(
repository, pathFragment, excludedPathFragments);
- for (PathFragment pkg : packagesUnderDirectory) {
- targetBuilder.merge(getTargetsInPackage(originalPattern,
- PackageIdentifier.create(repository, pkg),
- FilteringPolicies.NO_FILTER));
+
+ Iterable<PackageIdentifier> pkgIds = Iterables.transform(packagesUnderDirectory,
+ new Function<PathFragment, PackageIdentifier>() {
+ @Override
+ public PackageIdentifier apply(PathFragment path) {
+ return PackageIdentifier.create(repository, path);
+ }
+ });
+ for (ResolvedTargets<Target> targets : bulkGetTargetsInPackage(originalPattern, pkgIds,
+ FilteringPolicies.NO_FILTER).values()) {
+ targetBuilder.merge(targets);
}
// Perform the no-targets-found check before applying the filtering policy so we only return the
