Stream results of targets below directory to a callback rather than returning a ResolvedTargets set.
This is the first step in a series to allow processing large sets of targets in query target patterns via streaming batches rather than all at once. This should be a functional no-op.
--
MOS_MIGRATED_REVID=111609309
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 d3f758f..2641056 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
@@ -33,8 +33,11 @@
import com.google.devtools.build.lib.pkgcache.FilteringPolicy;
import com.google.devtools.build.lib.pkgcache.RecursivePackageProvider;
import com.google.devtools.build.lib.pkgcache.TargetPatternResolverUtil;
+import com.google.devtools.build.lib.util.BatchCallback;
import com.google.devtools.build.lib.vfs.PathFragment;
+import java.util.ArrayList;
+import java.util.List;
import java.util.Map;
/**
@@ -43,6 +46,9 @@
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 final RecursivePackageProvider recursivePackageProvider;
private final EventHandler eventHandler;
private final FilteringPolicy policy;
@@ -154,17 +160,20 @@
}
@Override
- public ResolvedTargets<Target> findTargetsBeneathDirectory(final RepositoryName repository,
- String originalPattern, String directory, boolean rulesOnly,
- ImmutableSet<String> excludedSubdirectories)
- throws TargetParsingException, InterruptedException {
+ public <E extends Exception> void findTargetsBeneathDirectory(
+ final RepositoryName repository,
+ String originalPattern,
+ String directory,
+ boolean rulesOnly,
+ ImmutableSet<String> excludedSubdirectories,
+ BatchCallback<Target, E> callback)
+ throws TargetParsingException, E, InterruptedException {
FilteringPolicy actualPolicy = rulesOnly
? FilteringPolicies.and(FilteringPolicies.RULES_ONLY, policy)
: policy;
ImmutableSet<PathFragment> excludedPathFragments =
TargetPatternResolverUtil.getPathFragments(excludedSubdirectories);
PathFragment pathFragment = TargetPatternResolverUtil.getPathFragment(directory);
- ResolvedTargets.Builder<Target> targetBuilder = ResolvedTargets.builder();
Iterable<PathFragment> packagesUnderDirectory =
recursivePackageProvider.getPackagesUnderDirectory(
repository, pathFragment, excludedPathFragments);
@@ -176,28 +185,30 @@
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
- // error if the input directory's subtree really contains no targets.
- if (targetBuilder.isEmpty()) {
- throw new TargetParsingException("no targets found beneath '" + pathFragment + "'");
- }
- ResolvedTargets<Target> prefilteredTargets = targetBuilder.build();
-
- ResolvedTargets.Builder<Target> filteredBuilder = ResolvedTargets.builder();
- if (prefilteredTargets.hasError()) {
- filteredBuilder.setError();
- }
- for (Target target : prefilteredTargets.getTargets()) {
- if (actualPolicy.shouldRetain(target, false)) {
- filteredBuilder.add(target);
+ boolean foundTarget = false;
+ // 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)) {
+ for (ResolvedTargets<Target> targets :
+ bulkGetTargetsInPackage(originalPattern, pkgIdBatch, FilteringPolicies.NO_FILTER)
+ .values()) {
+ List<Target> filteredTargets = new ArrayList<>(targets.getTargets().size());
+ 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);
+ }
+ }
+ callback.process(filteredTargets);
}
}
- return filteredBuilder.build();
+
+ if (!foundTarget) {
+ throw new TargetParsingException("no targets found beneath '" + pathFragment + "'");
+ }
}
}
