Query: move LabelVisitor to query package
LabelVisitor (and friends) are only used by BlazeQueryEnvironment, so
move them to live next to it, and reduce visibility to the package.
Also make a tiny refactoring in LabelVisitor to make the
VisitationAttributes inner class static.
This is in preparation for a rewrite of BlazeQueryEnvironment that
also requires changes to LabelVisitor.
PiperOrigin-RevId: 250129148
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 5557482..f30d40d 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
@@ -41,9 +41,7 @@
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.SilentCloseable;
import com.google.devtools.build.lib.query2.AbstractBlazeQueryEnvironment;
-import com.google.devtools.build.lib.query2.ErrorPrintingTargetEdgeErrorObserver;
import com.google.devtools.build.lib.query2.FakeLoadTarget;
-import com.google.devtools.build.lib.query2.LabelVisitor;
import com.google.devtools.build.lib.query2.engine.Callback;
import com.google.devtools.build.lib.query2.engine.DigraphQueryEvalResult;
import com.google.devtools.build.lib.query2.engine.MinDepthUniquifier;
diff --git a/src/main/java/com/google/devtools/build/lib/query2/ErrorPrintingTargetEdgeErrorObserver.java b/src/main/java/com/google/devtools/build/lib/query2/query/ErrorPrintingTargetEdgeErrorObserver.java
similarity index 83%
rename from src/main/java/com/google/devtools/build/lib/query2/ErrorPrintingTargetEdgeErrorObserver.java
rename to src/main/java/com/google/devtools/build/lib/query2/query/ErrorPrintingTargetEdgeErrorObserver.java
index 62acfd2..3aeabb7 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/ErrorPrintingTargetEdgeErrorObserver.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/query/ErrorPrintingTargetEdgeErrorObserver.java
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.devtools.build.lib.query2;
+package com.google.devtools.build.lib.query2.query;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.concurrent.ThreadSafety;
@@ -31,13 +31,11 @@
*/
@ThreadSafety.ConditionallyThreadSafe // condition: only call hasErrors
// once the visitation is complete.
-public class ErrorPrintingTargetEdgeErrorObserver extends TargetEdgeErrorObserver {
+class ErrorPrintingTargetEdgeErrorObserver extends TargetEdgeErrorObserver {
private final EventHandler eventHandler;
- /**
- * @param eventHandler eventHandler to route exceptions to as errors.
- */
+ /** @param eventHandler eventHandler to route exceptions to as errors. */
public ErrorPrintingTargetEdgeErrorObserver(EventHandler eventHandler) {
this.eventHandler = eventHandler;
}
@@ -45,8 +43,9 @@
@ThreadSafety.ThreadSafe
@Override
public void missingEdge(Target target, Label label, NoSuchThingException e) {
- eventHandler.handle(Event.error(TargetUtils.getLocationMaybe(target),
- TargetUtils.formatMissingEdge(target, label, e)));
+ eventHandler.handle(
+ Event.error(
+ TargetUtils.getLocationMaybe(target), TargetUtils.formatMissingEdge(target, label, e)));
super.missingEdge(target, label, e);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/query2/LabelVisitor.java b/src/main/java/com/google/devtools/build/lib/query2/query/LabelVisitor.java
similarity index 89%
rename from src/main/java/com/google/devtools/build/lib/query2/LabelVisitor.java
rename to src/main/java/com/google/devtools/build/lib/query2/query/LabelVisitor.java
index 5718d6a..19bb378 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/LabelVisitor.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/query/LabelVisitor.java
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.devtools.build.lib.query2;
+package com.google.devtools.build.lib.query2.query;
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
@@ -71,20 +71,22 @@
* similar must not be called until the concurrent phase is over, i.e. all external calls to visit()
* methods have completed.
*/
-public final class LabelVisitor {
+final class LabelVisitor {
+ private static final VisitationAttributes NONE = new VisitationAttributes(ImmutableList.of(), 0);
- /**
- * Attributes of a visitation which determine whether it is up-to-date or not.
- */
- private class VisitationAttributes {
- private Collection<Target> targetsToVisit;
- private boolean success = false;
- private int maxDepth = 0;
+ /** Attributes of a visitation which determine whether it is up-to-date or not. */
+ private static class VisitationAttributes {
+ private final Collection<Target> targetsToVisit;
+ private final int maxDepth;
+ private boolean success;
- /**
- * Returns true if and only if this visitation attribute is still up-to-date.
- */
- boolean current() {
+ VisitationAttributes(Collection<Target> targetsToVisit, int maxDepth) {
+ this.targetsToVisit = targetsToVisit;
+ this.maxDepth = maxDepth;
+ }
+
+ /** Returns true if and only if this visitation attribute is still up-to-date. */
+ boolean current(VisitationAttributes lastVisitation) {
return targetsToVisit.equals(lastVisitation.targetsToVisit)
&& maxDepth <= lastVisitation.maxDepth;
}
@@ -148,9 +150,7 @@
private VisitationAttributes lastVisitation;
- /**
- * Constant for limiting the permitted depth of recursion.
- */
+ /** Constant for limiting the permitted depth of recursion. */
private static final int RECURSION_LIMIT = 100;
/**
@@ -162,12 +162,12 @@
public LabelVisitor(
TargetProvider targetProvider, DependencyFilter edgeFilter, boolean useForkJoinPool) {
this.targetProvider = targetProvider;
- this.lastVisitation = new VisitationAttributes();
+ this.lastVisitation = NONE;
this.edgeFilter = edgeFilter;
this.useForkJoinPool = useForkJoinPool;
}
- public boolean syncWithVisitor(
+ public void syncWithVisitor(
ExtendedEventHandler eventHandler,
Collection<Target> targetsToVisit,
boolean keepGoing,
@@ -175,27 +175,19 @@
int maxDepth,
TargetEdgeObserver... observers)
throws InterruptedException {
- VisitationAttributes nextVisitation = new VisitationAttributes();
- nextVisitation.targetsToVisit = targetsToVisit;
- nextVisitation.maxDepth = maxDepth;
-
- if (!lastVisitation.success || !nextVisitation.current()) {
- try {
- nextVisitation.success = redoVisitation(eventHandler, nextVisitation, keepGoing,
- parallelThreads, maxDepth, observers);
- return nextVisitation.success;
- } finally {
- lastVisitation = nextVisitation;
- }
- } else {
- return true;
+ VisitationAttributes nextVisitation = new VisitationAttributes(targetsToVisit, maxDepth);
+ if (!lastVisitation.success || !nextVisitation.current(lastVisitation)) {
+ lastVisitation = nextVisitation;
+ lastVisitation.success =
+ redoVisitation(
+ eventHandler, targetsToVisit, keepGoing, parallelThreads, maxDepth, observers);
}
}
// Does a bounded transitive visitation starting at the given top-level targets.
private boolean redoVisitation(
ExtendedEventHandler eventHandler,
- VisitationAttributes visitation,
+ Iterable<Target> targetsToVisit,
boolean keepGoing,
int parallelThreads,
int maxDepth,
@@ -208,7 +200,7 @@
Throwable uncaught = null;
boolean result;
try {
- visitor.visitTargets(visitation.targetsToVisit);
+ visitor.visitTargets(targetsToVisit);
} catch (Throwable t) {
visitor.stopNewActions();
uncaught = t;
@@ -225,7 +217,7 @@
}
private class Visitor {
- private final static String THREAD_NAME = "LabelVisitor";
+ private static final String THREAD_NAME = "LabelVisitor";
private final ExecutorService executorService;
private final QuiescingExecutor executor;
@@ -285,7 +277,10 @@
}
private void enqueueTarget(
- final Target from, final Attribute attr, final Label label, final int depth,
+ final Target from,
+ final Attribute attr,
+ final Label label,
+ final int depth,
final int count) {
// Don't perform the targetProvider lookup if at the maximum depth already.
if (depth >= maxDepth) {
@@ -303,8 +298,12 @@
}
}
- private Runnable newVisitRunnable(final Target from, final Attribute attr, final Label label,
- final int depth, final int count) {
+ private Runnable newVisitRunnable(
+ final Target from,
+ final Attribute attr,
+ final Label label,
+ final int depth,
+ final int count) {
return () -> {
try {
try {
@@ -348,9 +347,7 @@
private void visitRule(final Rule rule, final int depth, final int count)
throws InterruptedException {
// Follow all labels defined by this rule:
- AggregatingAttributeMapper.of(rule)
- .visitLabels()
- .stream()
+ AggregatingAttributeMapper.of(rule).visitLabels().stream()
.filter(depEdge -> edgeFilter.apply(rule, depEdge.getAttribute()))
.forEach(
depEdge ->
@@ -374,9 +371,10 @@
throws InterruptedException {
if (target == null) {
throw new NullPointerException(
- String.format("'%s' attribute '%s'",
- from == null ? "(null)" : from.getLabel().toString(),
- attribute == null ? "(null)" : attribute.getName()));
+ String.format(
+ "'%s' attribute '%s'",
+ from == null ? "(null)" : from.getLabel().toString(),
+ attribute == null ? "(null)" : attribute.getName()));
}
if (depth > maxDepth) {
return;
diff --git a/src/main/java/com/google/devtools/build/lib/query2/TargetEdgeErrorObserver.java b/src/main/java/com/google/devtools/build/lib/query2/query/TargetEdgeErrorObserver.java
similarity index 68%
rename from src/main/java/com/google/devtools/build/lib/query2/TargetEdgeErrorObserver.java
rename to src/main/java/com/google/devtools/build/lib/query2/query/TargetEdgeErrorObserver.java
index 6704735..725ffcc 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/TargetEdgeErrorObserver.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/query/TargetEdgeErrorObserver.java
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.devtools.build.lib.query2;
+package com.google.devtools.build.lib.query2.query;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.concurrent.ThreadSafety;
@@ -23,22 +23,21 @@
import com.google.devtools.build.lib.pkgcache.TargetEdgeObserver;
/**
- * Record errors, such as missing package/target or rules containing errors,
- * encountered during visitation. Emit an error message upon encountering
- * missing edges
+ * Record errors, such as missing package/target or rules containing errors, encountered during
+ * visitation. Emit an error message upon encountering missing edges
*
- * The accessor {@link #hasErrors}) may not be called until the concurrent phase
- * is over, i.e. all external calls to visit() methods have completed.
+ * <p>The accessor {@link #hasErrors}) may not be called until the concurrent phase is over, i.e.
+ * all external calls to visit() methods have completed.
*
- * If you need to report errors to the console during visitation, use the
- * subclass {@link com.google.devtools.build.lib.query2.ErrorPrintingTargetEdgeErrorObserver}.
+ * <p>If you need to report errors to the console during visitation, use the subclass {@link
+ * ErrorPrintingTargetEdgeErrorObserver}.
*/
class TargetEdgeErrorObserver implements TargetEdgeObserver {
/**
- * True iff errors were encountered. Note, may be set to "true" during the
- * concurrent phase. Volatile, because it is assigned by worker threads and
- * read by the main thread without monitor synchronization.
+ * True iff errors were encountered. Note, may be set to "true" during the concurrent phase.
+ * Volatile, because it is assigned by worker threads and read by the main thread without monitor
+ * synchronization.
*/
private volatile boolean hasErrors = false;
@@ -56,13 +55,13 @@
}
/**
- * Returns true iff any errors (such as missing targets or packages, or rules
- * with errors) have been encountered during any work.
+ * Returns true iff any errors (such as missing targets or packages, or rules with errors) have
+ * been encountered during any work.
*
* <p>Not thread-safe; do not call during visitation.
*
- * @return true iff no errors (such as missing targets or packages, or rules
- * with errors) have been encountered during any work.
+ * @return true iff no errors (such as missing targets or packages, or rules with errors) have
+ * been encountered during any work.
*/
public boolean hasErrors() {
return hasErrors;
@@ -77,7 +76,7 @@
public void node(Target node) {
if (node.getPackage().containsErrors()
|| ((node instanceof Rule) && ((Rule) node).containsErrors())) {
- this.hasErrors = true; // Note, this is thread-safe.
+ this.hasErrors = true; // Note, this is thread-safe.
}
}
}
