Remove cost of iterating over a list of TargetEdgeObservers in LabelVisitor.
In a build with many genquery rules, this can account for a significant amount of cpu and garbage.
LabelVisitor now takes in a single TargetEdgeObserver. Callers can create a composite.
PiperOrigin-RevId: 318814266
diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/BlazeQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/query/BlazeQueryEnvironment.java
index d8700e4..4a27352 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/query/BlazeQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/query/BlazeQueryEnvironment.java
@@ -313,7 +313,6 @@
keepGoing,
loadingPhaseThreads,
maxDepth,
- errorObserver,
new GraphBuildingObserver());
}
@@ -379,16 +378,18 @@
dependencyFilter.apply(((Rule) from), attribute),
"Disallowed edge from LabelVisitor: %s --> %s", from, to);
makeEdge(from, to);
+ errorObserver.edge(from, attribute, to);
}
@Override
public void node(Target node) {
graph.createNode(node);
+ errorObserver.node(node);
}
@Override
public void missingEdge(Target target, Label to, NoSuchThingException e) {
- // No - op.
+ errorObserver.missingEdge(target, to, e);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/GraphlessBlazeQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/query/GraphlessBlazeQueryEnvironment.java
index d3ca5b4..8b97828 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/query/GraphlessBlazeQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/query/GraphlessBlazeQueryEnvironment.java
@@ -208,18 +208,21 @@
keepGoing,
loadingPhaseThreads,
maxDepth,
- errorObserver,
new TargetEdgeObserver() {
@Override
- public void edge(Target from, Attribute attribute, Target to) {}
+ public void edge(Target from, Attribute attribute, Target to) {
+ errorObserver.edge(from, attribute, to);
+ }
@Override
- public void missingEdge(
- @Nullable Target target, Label to, NoSuchThingException e) {}
+ public void missingEdge(@Nullable Target target, Label to, NoSuchThingException e) {
+ errorObserver.missingEdge(target, to, e);
+ }
@Override
public void node(Target node) {
result.add(node);
+ errorObserver.node(node);
}
});
}
diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/LabelVisitor.java b/src/main/java/com/google/devtools/build/lib/query2/query/LabelVisitor.java
index 879c14c..712df83 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/query/LabelVisitor.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/query/LabelVisitor.java
@@ -15,7 +15,6 @@
package com.google.devtools.build.lib.query2.query;
import com.google.common.base.Throwables;
-import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.util.concurrent.ThreadFactoryBuilder;
import com.google.devtools.build.lib.cmdline.Label;
@@ -166,14 +165,14 @@
boolean keepGoing,
int parallelThreads,
int maxDepth,
- TargetEdgeObserver... observers)
+ TargetEdgeObserver observer)
throws InterruptedException {
VisitationAttributes nextVisitation = new VisitationAttributes(targetsToVisit, maxDepth);
if (!lastVisitation.success || !nextVisitation.current(lastVisitation)) {
lastVisitation = nextVisitation;
lastVisitation.success =
redoVisitation(
- eventHandler, targetsToVisit, keepGoing, parallelThreads, maxDepth, observers);
+ eventHandler, targetsToVisit, keepGoing, parallelThreads, maxDepth, observer);
}
}
@@ -187,10 +186,10 @@
boolean keepGoing,
int parallelThreads,
int maxDepth,
- TargetEdgeObserver... observers)
+ TargetEdgeObserver observer)
throws InterruptedException {
lastVisitation = NONE;
- redoVisitation(eventHandler, targetsToVisit, keepGoing, parallelThreads, maxDepth, observers);
+ redoVisitation(eventHandler, targetsToVisit, keepGoing, parallelThreads, maxDepth, observer);
}
// Does a bounded transitive visitation starting at the given top-level targets.
@@ -200,11 +199,11 @@
boolean keepGoing,
int parallelThreads,
int maxDepth,
- TargetEdgeObserver... observers)
+ TargetEdgeObserver observer)
throws InterruptedException {
visitedTargets.clear();
- Visitor visitor = new Visitor(eventHandler, keepGoing, parallelThreads, maxDepth, observers);
+ Visitor visitor = new Visitor(eventHandler, keepGoing, parallelThreads, maxDepth, observer);
Throwable uncaught = null;
boolean result;
@@ -232,15 +231,17 @@
private final QuiescingExecutor executor;
private final ExtendedEventHandler eventHandler;
private final int maxDepth;
- private final Iterable<TargetEdgeObserver> observers;
- private final TargetEdgeErrorObserver errorObserver;
+
+ // Observers are stored individually instead of in a list to reduce iteration cost.
+ private final TargetEdgeObserver observer;
+ private final TargetEdgeErrorObserver errorObserver = new TargetEdgeErrorObserver();
Visitor(
ExtendedEventHandler eventHandler,
boolean keepGoing,
int parallelThreads,
int maxDepth,
- TargetEdgeObserver... observers) {
+ TargetEdgeObserver observer) {
if (parallelThreads > 1) {
this.executorService = NamedForkJoinPool.newNamedPool(THREAD_NAME, parallelThreads);
} else {
@@ -255,11 +256,7 @@
executorService, /*failFastOnException=*/ !keepGoing, ErrorClassifier.DEFAULT);
this.eventHandler = eventHandler;
this.maxDepth = maxDepth;
- this.errorObserver = new TargetEdgeErrorObserver();
- ImmutableList.Builder<TargetEdgeObserver> builder = ImmutableList.builder();
- builder.add(observers);
- builder.add(errorObserver);
- this.observers = builder.build();
+ this.observer = observer;
}
/**
@@ -314,11 +311,9 @@
final int count) {
return () -> {
try {
- try {
- visit(from, attr, targetProvider.getTarget(eventHandler, label), depth + 1, count);
- } catch (NoSuchThingException e) {
- observeError(from, label, e);
- }
+ visit(from, attr, targetProvider.getTarget(eventHandler, label), depth + 1, count);
+ } catch (NoSuchThingException e) {
+ observeError(from, label, e);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
@@ -419,22 +414,18 @@
}
private void observeEdge(Target from, Attribute attribute, Target to) {
- for (TargetEdgeObserver observer : observers) {
- observer.edge(from, attribute, to);
- }
+ observer.edge(from, attribute, to);
+ errorObserver.edge(from, attribute, to);
}
private void observeNode(Target target) {
- for (TargetEdgeObserver observer : observers) {
- observer.node(target);
- }
+ observer.node(target);
+ errorObserver.node(target);
}
- private void observeError(Target from, Label label, NoSuchThingException e)
- throws InterruptedException {
- for (TargetEdgeObserver observer : observers) {
- observer.missingEdge(from, label, e);
- }
+ private void observeError(Target from, Label label, NoSuchThingException e) {
+ observer.missingEdge(from, label, e);
+ errorObserver.missingEdge(from, label, e);
}
}
}
