Fix a minor bug where RRBuildFilesVisitor wasn't using an
inter-visitation-batch Uniquifier for the results.
Because SkyQueryEnvironment#BatchStreamedCallback (the [2nd-]most outer
Callback) internally uses a Uniquifier, this is merely a minor performance bug,
not a correctness bug.
RELNOTES: None
PiperOrigin-RevId: 234704160
diff --git a/src/main/java/com/google/devtools/build/lib/query2/ParallelSkyQueryUtils.java b/src/main/java/com/google/devtools/build/lib/query2/ParallelSkyQueryUtils.java
index df706da..a9a0aca 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/ParallelSkyQueryUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/ParallelSkyQueryUtils.java
@@ -31,7 +31,6 @@
import com.google.devtools.build.lib.query2.engine.QueryExpressionContext;
import com.google.devtools.build.lib.query2.engine.QueryUtil;
import com.google.devtools.build.lib.query2.engine.QueryUtil.AggregateAllCallback;
-import com.google.devtools.build.lib.query2.engine.Uniquifier;
import com.google.devtools.build.lib.skyframe.SkyFunctions;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.SkyKey;
@@ -54,7 +53,7 @@
public class ParallelSkyQueryUtils {
/** The maximum number of keys to visit at once. */
- @VisibleForTesting static final int VISIT_BATCH_SIZE = 10000;
+ @VisibleForTesting public static final int VISIT_BATCH_SIZE = 10000;
private ParallelSkyQueryUtils() {
}
@@ -167,11 +166,11 @@
Collection<PathFragment> fileIdentifiers,
QueryExpressionContext<Target> context,
Callback<Target> callback) throws QueryException, InterruptedException {
- Uniquifier<SkyKey> keyUniquifier = env.createSkyKeyUniquifier();
RBuildFilesVisitor visitor =
new RBuildFilesVisitor(
env,
- keyUniquifier,
+ /*visitUniquifier=*/ env.createSkyKeyUniquifier(),
+ /*resultUniquifier=*/ env.createSkyKeyUniquifier(),
context,
callback,
/*rdepFilter=*/ rdep ->
diff --git a/src/main/java/com/google/devtools/build/lib/query2/RBuildFilesVisitor.java b/src/main/java/com/google/devtools/build/lib/query2/RBuildFilesVisitor.java
index e1d9f87..7f89545 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/RBuildFilesVisitor.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/RBuildFilesVisitor.java
@@ -39,29 +39,39 @@
private static final SkyKey EXTERNAL_PACKAGE_KEY =
PackageValue.key(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER);
private final SkyQueryEnvironment env;
+ private final Uniquifier<SkyKey> resultUniquifier;
private final QueryExpressionContext<Target> context;
private final Function<SkyKey, Boolean> rdepFilter;
public RBuildFilesVisitor(
SkyQueryEnvironment env,
- Uniquifier<SkyKey> uniquifier,
+ Uniquifier<SkyKey> visitUniquifier,
+ Uniquifier<SkyKey> resultUniquifier,
QueryExpressionContext<Target> context,
Callback<Target> callback,
Function<SkyKey, Boolean> rdepFilter) {
- super(uniquifier, callback, ParallelSkyQueryUtils.VISIT_BATCH_SIZE, PROCESS_RESULTS_BATCH_SIZE);
+ super(
+ visitUniquifier,
+ callback,
+ ParallelSkyQueryUtils.VISIT_BATCH_SIZE,
+ PROCESS_RESULTS_BATCH_SIZE);
this.env = env;
+ this.resultUniquifier = resultUniquifier;
this.context = context;
this.rdepFilter = rdepFilter;
}
@Override
- protected Visit getVisitResult(Iterable<SkyKey> values) throws InterruptedException {
+ protected Visit getVisitResult(Iterable<SkyKey> values)
+ throws QueryException, InterruptedException {
Collection<Iterable<SkyKey>> reverseDeps = env.graph.getReverseDeps(values).values();
Set<SkyKey> keysToUseForResult = CompactHashSet.create();
Set<SkyKey> keysToVisitNext = CompactHashSet.create();
for (SkyKey rdep : Iterables.concat(reverseDeps)) {
if (rdep.functionName().equals(SkyFunctions.PACKAGE)) {
- keysToUseForResult.add(rdep);
+ if (resultUniquifier.unique(rdep)) {
+ keysToUseForResult.add(rdep);
+ }
// Every package has a dep on the external package, so we need to include those edges too.
if (rdep.equals(EXTERNAL_PACKAGE_KEY)) {
keysToVisitNext.add(rdep);
