Query performance: resolve simple patterns directly
Instead of depending on the TargetPatternFunction for simple patterns,
depend directly on the PackageValue and use that to resolve the targets
outside of Skyframe.
The problem with TPF is that it returns labels rather than targets, so
query has to turn the labels back to targets in order to be able to
process the results.
For a synthetic benchmark, we're seeing a significant improvement in
performance.
Before:
Cold 0m18.469s
Hot 0m6.045s
After:
Cold 0m15.778s
Hot 0m4.892s
PiperOrigin-RevId: 248311593
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternsResultBuilder.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternsResultBuilder.java
index db4f32e..933c061 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternsResultBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternsResultBuilder.java
@@ -14,11 +14,12 @@
package com.google.devtools.build.lib.skyframe;
import com.google.common.base.Preconditions;
+import com.google.common.base.Predicates;
+import com.google.common.collect.Iterables;
import com.google.common.collect.Maps;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.ResolvedTargets;
-import com.google.devtools.build.lib.cmdline.TargetParsingException;
import com.google.devtools.build.lib.packages.NoSuchTargetException;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.Target;
@@ -31,22 +32,21 @@
* This class encapsulates logic behind computing final target set based on separate results from a
* list of target patterns (eg, //foo:all -//bar/... //foo:test).
*/
-abstract class TargetPatternsResultBuilder {
- private Map<PackageIdentifier, Package> packages = null;
- private boolean hasError = false;
+class TargetPatternsResultBuilder {
+ private final ResolvedTargets.Builder<Label> resolvedLabelsBuilder = ResolvedTargets.builder();
+ private Map<PackageIdentifier, Package> packages;
+ private boolean hasError;
- /**
- * Sets that there was an error, during evaluation.
- */
+ /** Sets that there was an error, during evaluation. */
public void setError() {
hasError = true;
}
/** Returns final set of targets and sets error flag if required. */
- public ResolvedTargets<Target> build(WalkableGraph walkableGraph)
- throws TargetParsingException, InterruptedException {
+ public ResolvedTargets<Target> build(WalkableGraph walkableGraph) throws InterruptedException {
precomputePackages(walkableGraph);
- ResolvedTargets.Builder<Target> resolvedTargetsBuilder = buildInternal();
+ ResolvedTargets.Builder<Target> resolvedTargetsBuilder =
+ transformLabelsIntoTargets(resolvedLabelsBuilder.build());
if (hasError) {
resolvedTargetsBuilder.setError();
}
@@ -58,7 +58,7 @@
* method is using information about packages, so {@link #precomputePackages} has to be called
* before this method.
*/
- protected ResolvedTargets.Builder<Target> transformLabelsIntoTargets(
+ private ResolvedTargets.Builder<Target> transformLabelsIntoTargets(
ResolvedTargets<Label> resolvedLabels) {
// precomputePackages has to be called before this method.
ResolvedTargets.Builder<Target> resolvedTargetsBuilder = ResolvedTargets.builder();
@@ -107,24 +107,19 @@
.getPackage();
}
- /**
- * Adds the result from expansion of positive target pattern (eg, "//foo:all").
- */
- abstract void addLabelsOfNegativePattern(ResolvedTargets<Label> labels);
+ /** Adds the result from expansion of positive target pattern (eg, "//foo:all"). */
+ void addLabelsOfNegativePattern(ResolvedTargets<Label> labels) {
+ resolvedLabelsBuilder.filter(Predicates.not(Predicates.in(labels.getTargets())));
+ }
- /**
- * Adds the result from expansion of negative target pattern (eg, "-//foo:all").
- */
- abstract void addLabelsOfPositivePattern(ResolvedTargets<Label> labels);
+ /** Adds the result from expansion of negative target pattern (eg, "-//foo:all"). */
+ void addLabelsOfPositivePattern(ResolvedTargets<Label> labels) {
+ resolvedLabelsBuilder.merge(labels);
+ }
- /**
- * Returns {@code ResolvedTargets.Builder<Target>} with final set of targets. Note that this
- * method doesn't set error flag in result.
- */
- abstract ResolvedTargets.Builder<Target> buildInternal() throws TargetParsingException;
-
- /**
- * Returns target labels from all individual results.
- */
- protected abstract Iterable<Label> getLabels();
+ /** Returns target labels from all individual results. */
+ private Iterable<Label> getLabels() {
+ ResolvedTargets<Label> resolvedLabels = resolvedLabelsBuilder.build();
+ return Iterables.concat(resolvedLabels.getTargets(), resolvedLabels.getFilteredTargets());
+ }
}