Parallelize Package retrieval during Sky-query operation. To maintain type-safety, we now must pass in the exception type of the callback.
--
MOS_MIGRATED_REVID=113313312
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java
index 5617c94..8b49a2a 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java
@@ -143,9 +143,9 @@
* Evaluates the current target pattern and returns the result.
*/
public <T, E extends Exception> void eval(
- TargetPatternResolver<T> resolver, BatchCallback<T, E> callback)
+ TargetPatternResolver<T> resolver, BatchCallback<T, E> callback, Class<E> exceptionClass)
throws TargetParsingException, E, InterruptedException {
- eval(resolver, ImmutableSet.<PathFragment>of(), callback);
+ eval(resolver, ImmutableSet.<PathFragment>of(), callback, exceptionClass);
}
/**
@@ -158,7 +158,7 @@
public abstract <T, E extends Exception> void eval(
TargetPatternResolver<T> resolver,
ImmutableSet<PathFragment> excludedSubdirectories,
- BatchCallback<T, E> callback)
+ BatchCallback<T, E> callback, Class<E> exceptionClass)
throws TargetParsingException, E, InterruptedException;
/**
@@ -210,7 +210,7 @@
public <T, E extends Exception> void eval(
TargetPatternResolver<T> resolver,
ImmutableSet<PathFragment> excludedSubdirectories,
- BatchCallback<T, E> callback)
+ BatchCallback<T, E> callback, Class<E> exceptionClass)
throws TargetParsingException, E, InterruptedException {
Preconditions.checkArgument(excludedSubdirectories.isEmpty(),
"Target pattern \"%s\" of type %s cannot be evaluated with excluded subdirectories: %s.",
@@ -264,7 +264,7 @@
public <T, E extends Exception> void eval(
TargetPatternResolver<T> resolver,
ImmutableSet<PathFragment> excludedSubdirectories,
- BatchCallback<T, E> callback)
+ BatchCallback<T, E> callback, Class<E> exceptionClass)
throws TargetParsingException, E, InterruptedException {
Preconditions.checkArgument(excludedSubdirectories.isEmpty(),
"Target pattern \"%s\" of type %s cannot be evaluated with excluded subdirectories: %s.",
@@ -353,7 +353,7 @@
public <T, E extends Exception> void eval(
TargetPatternResolver<T> resolver,
ImmutableSet<PathFragment> excludedSubdirectories,
- BatchCallback<T, E> callback)
+ BatchCallback<T, E> callback, Class<E> exceptionClass)
throws TargetParsingException, E, InterruptedException {
Preconditions.checkArgument(excludedSubdirectories.isEmpty(),
"Target pattern \"%s\" of type %s cannot be evaluated with excluded subdirectories: %s.",
@@ -463,7 +463,7 @@
public <T, E extends Exception> void eval(
TargetPatternResolver<T> resolver,
ImmutableSet<PathFragment> excludedSubdirectories,
- BatchCallback<T, E> callback)
+ BatchCallback<T, E> callback, Class<E> exceptionClass)
throws TargetParsingException, E, InterruptedException {
resolver.findTargetsBeneathDirectory(
directory.getRepository(),
@@ -471,7 +471,8 @@
directory.getPackageFragment().getPathString(),
rulesOnly,
excludedSubdirectories,
- callback);
+ callback,
+ exceptionClass);
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPatternResolver.java b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPatternResolver.java
index 05c914a..8dc0f0a 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPatternResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPatternResolver.java
@@ -79,6 +79,7 @@
* @param excludedSubdirectories a set of transitive subdirectories beneath {@code directory}
* to ignore
* @param callback the callback to receive the result, possibly in multiple batches.
+ * @param exceptionClass The class type of the parameterized exception.
* @throws TargetParsingException under implementation-specific failure conditions
*/
<E extends Exception> void findTargetsBeneathDirectory(
@@ -87,7 +88,7 @@
String directory,
boolean rulesOnly,
ImmutableSet<PathFragment> excludedSubdirectories,
- BatchCallback<T, E> callback)
+ BatchCallback<T, E> callback, Class<E> exceptionClass)
throws TargetParsingException, E, InterruptedException;
/**
diff --git a/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
index 363f5d5..f5ffe54 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
@@ -25,6 +25,7 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.Multimap;
import com.google.common.collect.Sets;
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
@@ -83,6 +84,8 @@
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
import java.util.logging.Logger;
import javax.annotation.Nullable;
@@ -418,13 +421,17 @@
.argument());
GraphBackedRecursivePackageProvider provider =
new GraphBackedRecursivePackageProvider(graph, universeTargetPatternKeys, pkgPath);
+
+ ExecutorService threadPool = Executors.newFixedThreadPool(
+ Runtime.getRuntime().availableProcessors(),
+ new ThreadFactoryBuilder().setNameFormat("GetPackages-%d").build());
RecursivePackageProviderBackedTargetPatternResolver resolver =
new RecursivePackageProviderBackedTargetPatternResolver(
- provider, eventHandler, targetPatternKey.getPolicy());
+ provider, eventHandler, targetPatternKey.getPolicy(), threadPool);
TargetPattern parsedPattern = targetPatternKey.getParsedPattern();
FilteringBatchingUniquifyingCallback wrapper =
new FilteringBatchingUniquifyingCallback(callback);
- parsedPattern.eval(resolver, wrapper);
+ parsedPattern.eval(resolver, wrapper, QueryException.class);
wrapper.processLastPending();
} catch (TargetParsingException e) {
reportBuildFileError(owner, e.getMessage());
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 f5c3ada..60221ad 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
@@ -27,6 +27,7 @@
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.cmdline.TargetPattern;
import com.google.devtools.build.lib.cmdline.TargetPattern.Type;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.BuildFileNotFoundException;
@@ -55,6 +56,7 @@
* SkyQueryEnvironment} to look up the packages and targets matching the universe that's been
* preloaded in {@code graph}.
*/
+@ThreadSafe
public final class GraphBackedRecursivePackageProvider implements RecursivePackageProvider {
private final WalkableGraph graph;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java
index f98a01d..781c001 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java
@@ -82,8 +82,8 @@
TargetPattern parsedPattern = patternKey.getParsedPattern();
DepsOfPatternPreparer preparer = new DepsOfPatternPreparer(env, pkgPath.get());
ImmutableSet<PathFragment> excludedSubdirectories = patternKey.getExcludedSubdirectories();
- parsedPattern.<Void, RuntimeException>eval(
- preparer, excludedSubdirectories, NullCallback.<Void>instance());
+ parsedPattern.eval(
+ preparer, excludedSubdirectories, NullCallback.<Void>instance(), RuntimeException.class);
} catch (TargetParsingException e) {
throw new PrepareDepsOfPatternFunctionException(e);
} catch (MissingDepException e) {
@@ -216,7 +216,7 @@
String directory,
boolean rulesOnly,
ImmutableSet<PathFragment> excludedSubdirectories,
- BatchCallback<Void, E> callback)
+ BatchCallback<Void, E> callback, Class<E> exceptionClass)
throws TargetParsingException, E, InterruptedException {
FilteringPolicy policy =
rulesOnly ? FilteringPolicies.RULES_ONLY : FilteringPolicies.NO_FILTER;
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 b24ee7a..65a737e 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
@@ -16,6 +16,7 @@
import static com.google.devtools.build.lib.pkgcache.FilteringPolicies.NO_FILTER;
import com.google.common.base.Function;
+import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
@@ -25,6 +26,8 @@
import com.google.devtools.build.lib.cmdline.ResolvedTargets;
import com.google.devtools.build.lib.cmdline.TargetParsingException;
import com.google.devtools.build.lib.cmdline.TargetPatternResolver;
+import com.google.devtools.build.lib.concurrent.ExecutorUtil;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.NoSuchPackageException;
@@ -41,27 +44,34 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
/**
* A {@link TargetPatternResolver} backed by a {@link RecursivePackageProvider}.
*/
+@ThreadCompatible
public class RecursivePackageProviderBackedTargetPatternResolver
implements TargetPatternResolver<Target> {
// TODO(janakr): Move this to a more generic place and unify with SkyQueryEnvironment's value?
- private static final int MAX_PACKAGES_BULK_GET = 10000;
+ private static final int MAX_PACKAGES_BULK_GET = 1000;
private final RecursivePackageProvider recursivePackageProvider;
private final EventHandler eventHandler;
private final FilteringPolicy policy;
+ private final ExecutorService executor;
public RecursivePackageProviderBackedTargetPatternResolver(
RecursivePackageProvider recursivePackageProvider,
EventHandler eventHandler,
- FilteringPolicy policy) {
+ FilteringPolicy policy,
+ ExecutorService executor) {
this.recursivePackageProvider = recursivePackageProvider;
this.eventHandler = eventHandler;
this.policy = policy;
+ this.executor = executor;
}
@Override
@@ -164,13 +174,13 @@
@Override
public <E extends Exception> void findTargetsBeneathDirectory(
final RepositoryName repository,
- String originalPattern,
+ final String originalPattern,
String directory,
boolean rulesOnly,
ImmutableSet<PathFragment> excludedSubdirectories,
- BatchCallback<Target, E> callback)
+ final BatchCallback<Target, E> callback, Class<E> exceptionClass)
throws TargetParsingException, E, InterruptedException {
- FilteringPolicy actualPolicy = rulesOnly
+ final FilteringPolicy actualPolicy = rulesOnly
? FilteringPolicies.and(FilteringPolicies.RULES_ONLY, policy)
: policy;
PathFragment pathFragment = TargetPatternResolverUtil.getPathFragment(directory);
@@ -185,29 +195,64 @@
return PackageIdentifier.create(repository, path);
}
});
- boolean foundTarget = false;
+ final AtomicBoolean foundTarget = new AtomicBoolean(false);
+ final AtomicReference<InterruptedException> interrupt = new AtomicReference<>();
+ final AtomicReference<TargetParsingException> parsingException = new AtomicReference<>();
+ final AtomicReference<Exception> genericException = new AtomicReference<>();
+
+ final Object callbackLock = new Object();
+
// For very large sets of packages, we may not want to process all of them at once, so we split
// into batches.
- for (Iterable<PackageIdentifier> pkgIdBatch :
- Iterables.partition(pkgIds, MAX_PACKAGES_BULK_GET)) {
+ try {
+ for (final Iterable<PackageIdentifier> pkgIdBatch :
+ Iterables.partition(pkgIds, MAX_PACKAGES_BULK_GET)) {
+ executor.execute(new Runnable() {
+ @Override
+ public void run() {
+ Iterable<ResolvedTargets<Target>> resolvedTargets = null;
+ try {
+ resolvedTargets =
+ bulkGetTargetsInPackage(originalPattern, pkgIdBatch, NO_FILTER).values();
+ } catch (InterruptedException e) {
+ interrupt.compareAndSet(null, e);
+ return;
+ } catch (TargetParsingException e) {
+ parsingException.compareAndSet(null, e);
+ }
- Iterable<ResolvedTargets<Target>> resolvedTargets =
- bulkGetTargetsInPackage(originalPattern, pkgIdBatch, NO_FILTER).values();
- List<Target> filteredTargets = new ArrayList<>(calculateSize(resolvedTargets));
- for (ResolvedTargets<Target> targets : resolvedTargets) {
- for (Target target : targets.getTargets()) {
- // Perform the no-targets-found check before applying the filtering policy so we only
- // return the error if the input directory's subtree really contains no targets.
- foundTarget = true;
- if (actualPolicy.shouldRetain(target, false)) {
- filteredTargets.add(target);
+ List<Target> filteredTargets = new ArrayList<>(calculateSize(resolvedTargets));
+ for (ResolvedTargets<Target> targets : resolvedTargets) {
+ for (Target target : targets.getTargets()) {
+ // Perform the no-targets-found check before applying the filtering policy so we
+ // only return the error if the input directory's subtree really contains no
+ // targets.
+ foundTarget.set(true);
+ if (actualPolicy.shouldRetain(target, false)) {
+ filteredTargets.add(target);
+ }
+ }
+ }
+ try {
+ synchronized (callbackLock) {
+ callback.process(filteredTargets);
+ }
+ } catch (InterruptedException e) {
+ interrupt.compareAndSet(null, e);
+ } catch (Exception e) {
+ genericException.compareAndSet(e, null);
+ }
}
- }
+ });
}
- callback.process(filteredTargets);
+ } finally {
+ ExecutorUtil.interruptibleShutdown(executor);
}
- if (!foundTarget) {
+ Throwables.propagateIfInstanceOf(interrupt.get(), InterruptedException.class);
+ Throwables.propagateIfInstanceOf(parsingException.get(), TargetParsingException.class);
+ Throwables.propagateIfPossible(genericException.get(), exceptionClass);
+ if (!foundTarget.get()) {
throw new TargetParsingException("no targets found beneath '" + pathFragment + "'");
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java
index 58c32ce..32f64e8 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java
@@ -15,6 +15,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
+import com.google.common.util.concurrent.MoreExecutors;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.ResolvedTargets;
import com.google.devtools.build.lib.cmdline.TargetParsingException;
@@ -54,7 +55,7 @@
new EnvironmentBackedRecursivePackageProvider(env);
RecursivePackageProviderBackedTargetPatternResolver resolver =
new RecursivePackageProviderBackedTargetPatternResolver(provider, env.getListener(),
- patternKey.getPolicy());
+ patternKey.getPolicy(), MoreExecutors.newDirectExecutorService());
TargetPattern parsedPattern = patternKey.getParsedPattern();
ImmutableSet<PathFragment> excludedSubdirectories = patternKey.getExcludedSubdirectories();
final Set<Target> results = CompactHashSet.create();
@@ -65,7 +66,7 @@
Iterables.addAll(results, partialResult);
}
};
- parsedPattern.eval(resolver, excludedSubdirectories, callback);
+ parsedPattern.eval(resolver, excludedSubdirectories, callback, RuntimeException.class);
resolvedTargets = ResolvedTargets.<Target>builder().addAll(results).build();
} catch (TargetParsingException e) {
throw new TargetPatternFunctionException(e);
diff --git a/src/main/java/com/google/devtools/build/skyframe/WalkableGraph.java b/src/main/java/com/google/devtools/build/skyframe/WalkableGraph.java
index 75c347f..fd43a07 100644
--- a/src/main/java/com/google/devtools/build/skyframe/WalkableGraph.java
+++ b/src/main/java/com/google/devtools/build/skyframe/WalkableGraph.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.skyframe;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.events.EventHandler;
import java.util.Collection;
@@ -24,6 +25,7 @@
* Read-only graph that exposes the dependents, dependencies (reverse dependents), and value and
* exception (if any) of a given node.
*/
+@ThreadSafe
public interface WalkableGraph {
/**
