Refactor QueryableGraph and ThinNodeQueryableGraph to be independent interfaces, in preparation for further changes.
--
MOS_MIGRATED_REVID=126924789
diff --git a/src/main/java/com/google/devtools/build/skyframe/DelegatingNodeEntry.java b/src/main/java/com/google/devtools/build/skyframe/DelegatingNodeEntry.java
index 8e9ddb6..d714924 100644
--- a/src/main/java/com/google/devtools/build/skyframe/DelegatingNodeEntry.java
+++ b/src/main/java/com/google/devtools/build/skyframe/DelegatingNodeEntry.java
@@ -157,22 +157,22 @@
@Override
public Iterable<SkyKey> getDirectDeps() {
- return getThinDelegate().getDirectDeps();
+ return getDelegate().getDirectDeps();
}
@Override
public void removeReverseDep(SkyKey reverseDep) {
- getThinDelegate().removeReverseDep(reverseDep);
+ getDelegate().removeReverseDep(reverseDep);
}
@Override
public void removeInProgressReverseDep(SkyKey reverseDep) {
- getThinDelegate().removeInProgressReverseDep(reverseDep);
+ getDelegate().removeInProgressReverseDep(reverseDep);
}
@Override
public Iterable<SkyKey> getReverseDeps() {
- return getThinDelegate().getReverseDeps();
+ return getDelegate().getReverseDeps();
}
@Override
diff --git a/src/main/java/com/google/devtools/build/skyframe/DirtiableGraph.java b/src/main/java/com/google/devtools/build/skyframe/DeletableGraph.java
similarity index 82%
rename from src/main/java/com/google/devtools/build/skyframe/DirtiableGraph.java
rename to src/main/java/com/google/devtools/build/skyframe/DeletableGraph.java
index fe7e88b..d091f08 100644
--- a/src/main/java/com/google/devtools/build/skyframe/DirtiableGraph.java
+++ b/src/main/java/com/google/devtools/build/skyframe/DeletableGraph.java
@@ -19,13 +19,11 @@
* Interface for classes that need to remove values from graph. Currently just used by {@link
* EagerInvalidator}.
*
- * <p>This class is not intended for direct use, and is only exposed as public for use in
- * evaluation implementations outside of this package.
+ * <p>This class is not intended for direct use, and is only exposed as public for use in evaluation
+ * implementations outside of this package.
*/
@ThreadSafe
-public interface DirtiableGraph extends QueryableGraph {
- /**
- * Remove the value with given name from the graph.
- */
+public interface DeletableGraph {
+ /** Remove the value with given name from the graph. */
void remove(SkyKey key);
}
diff --git a/src/main/java/com/google/devtools/build/skyframe/EagerInvalidator.java b/src/main/java/com/google/devtools/build/skyframe/EagerInvalidator.java
index bb6d2b2..b7c1ee9 100644
--- a/src/main/java/com/google/devtools/build/skyframe/EagerInvalidator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/EagerInvalidator.java
@@ -43,9 +43,14 @@
* long as the full upward transitive closure of the nodes is specified for deletion, the graph
* remains consistent.
*/
- public static void delete(DirtiableGraph graph, Iterable<SkyKey> diff,
- EvaluationProgressReceiver invalidationReceiver, InvalidationState state,
- boolean traverseGraph, DirtyKeyTracker dirtyKeyTracker) throws InterruptedException {
+ public static void delete(
+ InMemoryGraph graph,
+ Iterable<SkyKey> diff,
+ EvaluationProgressReceiver invalidationReceiver,
+ InvalidationState state,
+ boolean traverseGraph,
+ DirtyKeyTracker dirtyKeyTracker)
+ throws InterruptedException {
DeletingNodeVisitor visitor =
createDeletingVisitorIfNeeded(
graph, diff, invalidationReceiver, state, traverseGraph, dirtyKeyTracker);
@@ -56,7 +61,7 @@
@Nullable
static DeletingNodeVisitor createDeletingVisitorIfNeeded(
- DirtiableGraph graph,
+ InMemoryGraph graph,
Iterable<SkyKey> diff,
EvaluationProgressReceiver invalidationReceiver,
InvalidationState state,
@@ -70,7 +75,7 @@
@Nullable
static DirtyingNodeVisitor createInvalidatingVisitorIfNeeded(
- ThinNodeQueryableGraph graph,
+ InvalidatableGraph graph,
Iterable<SkyKey> diff,
EvaluationProgressReceiver invalidationReceiver,
InvalidationState state,
@@ -84,7 +89,7 @@
@Nullable
private static DirtyingNodeVisitor createInvalidatingVisitorIfNeeded(
- ThinNodeQueryableGraph graph,
+ InvalidatableGraph graph,
Iterable<SkyKey> diff,
EvaluationProgressReceiver invalidationReceiver,
InvalidationState state,
@@ -110,7 +115,7 @@
* an executor constructed with the provided factory.
*/
public static void invalidate(
- ThinNodeQueryableGraph graph,
+ InvalidatableGraph graph,
Iterable<SkyKey> diff,
EvaluationProgressReceiver invalidationReceiver,
InvalidationState state,
@@ -130,7 +135,7 @@
* the provided {@link ForkJoinPool}.
*/
public static void invalidate(
- ThinNodeQueryableGraph graph,
+ InvalidatableGraph graph,
Iterable<SkyKey> diff,
EvaluationProgressReceiver invalidationReceiver,
InvalidationState state,
@@ -154,11 +159,9 @@
}
}
- /**
- * Invalidates given values and their upward transitive closure in the graph.
- */
+ /** Invalidates given values and their upward transitive closure in the graph. */
public static void invalidate(
- DirtiableGraph graph,
+ InvalidatableGraph graph,
Iterable<SkyKey> diff,
EvaluationProgressReceiver invalidationReceiver,
InvalidationState state,
diff --git a/src/main/java/com/google/devtools/build/skyframe/EvaluableGraph.java b/src/main/java/com/google/devtools/build/skyframe/EvaluableGraph.java
index 7527146..b3d8bd0 100644
--- a/src/main/java/com/google/devtools/build/skyframe/EvaluableGraph.java
+++ b/src/main/java/com/google/devtools/build/skyframe/EvaluableGraph.java
@@ -22,7 +22,7 @@
* single version of the graph.
*/
@ThreadSafe
-interface EvaluableGraph extends QueryableGraph {
+interface EvaluableGraph extends QueryableGraph, DeletableGraph {
/**
* Like {@link QueryableGraph#getBatch}, except it creates a new node for each key not already
* present in the graph. Thus, the returned map will have an entry for each key in {@code keys}.
diff --git a/src/main/java/com/google/devtools/build/skyframe/InMemoryGraph.java b/src/main/java/com/google/devtools/build/skyframe/InMemoryGraph.java
index 02735c0..5d508a9 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InMemoryGraph.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryGraph.java
@@ -16,7 +16,7 @@
import java.util.Map;
/** {@link ProcessableGraph} that exposes the contents of the entire graph. */
-interface InMemoryGraph extends ProcessableGraph {
+interface InMemoryGraph extends ProcessableGraph, InvalidatableGraph {
/**
* Returns a read-only live view of the nodes in the graph. All node are included. Dirty values
* include their Node value. Values in error have a null value.
diff --git a/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java
index a5e22221..5d853b7 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java
@@ -284,9 +284,8 @@
}
@Override
- public void injectGraphTransformerForTesting(
- Function<ThinNodeQueryableGraph, ProcessableGraph> transformer) {
- this.graph = (InMemoryGraph) transformer.apply(this.graph);
+ public void injectGraphTransformerForTesting(GraphTransformerForTesting transformer) {
+ this.graph = transformer.transform(this.graph);
}
public ProcessableGraph getGraphForTesting() {
diff --git a/src/main/java/com/google/devtools/build/skyframe/ThinNodeQueryableGraph.java b/src/main/java/com/google/devtools/build/skyframe/InvalidatableGraph.java
similarity index 78%
rename from src/main/java/com/google/devtools/build/skyframe/ThinNodeQueryableGraph.java
rename to src/main/java/com/google/devtools/build/skyframe/InvalidatableGraph.java
index eeb3ef5..2437407 100644
--- a/src/main/java/com/google/devtools/build/skyframe/ThinNodeQueryableGraph.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InvalidatableGraph.java
@@ -17,20 +17,14 @@
import java.util.Map;
-import javax.annotation.Nullable;
-
/**
- * A graph that exposes thin representations of its entries and structure, for use by classes that
- * must traverse it, but not read its entries' values.
+ * A graph that exposes thin representations of its entries and structure, for use during
+ * invalidation.
+ *
+ * <p>Public only for use in alternative graph implementations.
*/
@ThreadSafe
-public interface ThinNodeQueryableGraph {
- /**
- * Returns the thin node with the given name, or {@code null} if the node does not exist.
- */
- @Nullable
- ThinNodeEntry get(SkyKey key);
-
+public interface InvalidatableGraph {
/**
* Fetches all the given thin nodes. Returns a map {@code m} such that, for all {@code k} in
* {@code keys}, {@code m.get(k).equals(e)} iff {@code get(k) == e} and {@code e != null}, and
diff --git a/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java b/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java
index 6eec236..1fe90d1 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java
@@ -62,7 +62,7 @@
*
* <p>This is intended only for use in alternative {@code MemoizingEvaluator} implementations.
*/
-public abstract class InvalidatingNodeVisitor<TGraph extends ThinNodeQueryableGraph> {
+public abstract class InvalidatingNodeVisitor<TGraph extends InvalidatableGraph> {
// Default thread count is equal to the number of cores to exploit
// that level of hardware parallelism, since invalidation should be CPU-bound.
@@ -236,13 +236,13 @@
}
/** A node-deleting implementation. */
- static class DeletingNodeVisitor extends InvalidatingNodeVisitor<DirtiableGraph> {
+ static class DeletingNodeVisitor extends InvalidatingNodeVisitor<InMemoryGraph> {
private final Set<SkyKey> visited = Sets.newConcurrentHashSet();
private final boolean traverseGraph;
DeletingNodeVisitor(
- DirtiableGraph graph,
+ InMemoryGraph graph,
EvaluationProgressReceiver invalidationReceiver,
InvalidationState state,
boolean traverseGraph,
@@ -338,7 +338,7 @@
}
/** A node-dirtying implementation. */
- static class DirtyingNodeVisitor extends InvalidatingNodeVisitor<ThinNodeQueryableGraph> {
+ static class DirtyingNodeVisitor extends InvalidatingNodeVisitor<InvalidatableGraph> {
private final Set<SkyKey> changed =
Collections.newSetFromMap(
@@ -351,7 +351,7 @@
private final boolean supportInterruptions;
protected DirtyingNodeVisitor(
- ThinNodeQueryableGraph graph,
+ InvalidatableGraph graph,
EvaluationProgressReceiver invalidationReceiver,
InvalidationState state,
DirtyKeyTracker dirtyKeyTracker,
@@ -365,7 +365,7 @@
* passing {@code false} for {@param supportInterruptions}.
*/
protected DirtyingNodeVisitor(
- ThinNodeQueryableGraph graph,
+ InvalidatableGraph graph,
EvaluationProgressReceiver invalidationReceiver,
InvalidationState state,
DirtyKeyTracker dirtyKeyTracker,
diff --git a/src/main/java/com/google/devtools/build/skyframe/MemoizingEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/MemoizingEvaluator.java
index 269ea09..4d5d21c 100644
--- a/src/main/java/com/google/devtools/build/skyframe/MemoizingEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/MemoizingEvaluator.java
@@ -14,7 +14,6 @@
package com.google.devtools.build.skyframe;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Function;
import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.collect.nestedset.NestedSetVisitor;
@@ -126,19 +125,18 @@
/**
* Tests that want finer control over the graph being used may provide a {@code transformer} here.
- * This {@code transformer} will be applied to the graph for each invalidation/evaluation. While
- * the graph returned by {@code transformer#apply} must technically be a {@link ProcessableGraph},
- * if a {@link ThinNodeQueryableGraph} was given as the argument to {@code transformer#apply},
- * then only the methods in {@link ThinNodeQueryableGraph} will be called on the returned graph,
- * in other words it will be treated as a {@link ThinNodeQueryableGraph}. Thus, the returned graph
- * is free not to actually implement the remaining methods in {@link ProcessableGraph} in that
- * case.
- *
- * <p>Similarly, if the argument to {@code transformer#apply} is an {@link InMemoryGraph}, then
- * the resulting graph must be an {@link InMemoryGraph}.
- * */
- void injectGraphTransformerForTesting(
- Function<ThinNodeQueryableGraph, ProcessableGraph> transformer);
+ * This {@code transformer} will be applied to the graph for each invalidation/evaluation.
+ */
+ void injectGraphTransformerForTesting(GraphTransformerForTesting transformer);
+
+ /** Transforms a graph, possibly injecting other functionality. */
+ interface GraphTransformerForTesting {
+ InMemoryGraph transform(InMemoryGraph graph);
+
+ InvalidatableGraph transform(InvalidatableGraph graph);
+
+ ProcessableGraph transform(ProcessableGraph graph);
+ }
/**
* Write the graph to the output stream. Not necessarily thread-safe. Use only for debugging
diff --git a/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java b/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java
index 7b38324..8b7b091 100644
--- a/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java
+++ b/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java
@@ -83,6 +83,37 @@
@ThreadSafe
SkyValue getValue();
+ /**
+ * Returns an immutable iterable of the direct deps of this node. This method may only be called
+ * after the evaluation of this node is complete.
+ *
+ * <p>This method is not very efficient, but is only be called in limited circumstances -- when
+ * the node is about to be deleted, or when the node is expected to have no direct deps (in which
+ * case the overhead is not so bad). It should not be called repeatedly for the same node, since
+ * each call takes time proportional to the number of direct deps of the node.
+ */
+ @ThreadSafe
+ Iterable<SkyKey> getDirectDeps();
+
+ /** Removes a reverse dependency. */
+ @ThreadSafe
+ void removeReverseDep(SkyKey reverseDep);
+
+ /**
+ * Removes a reverse dependency.
+ *
+ * <p>May only be called if this entry is not done (i.e. {@link #isDone} is false) and {@param
+ * reverseDep} is present in {@link #getReverseDeps}
+ */
+ @ThreadSafe
+ void removeInProgressReverseDep(SkyKey reverseDep);
+
+ /**
+ * Returns a copy of the set of reverse dependencies. Note that this introduces a potential
+ * check-then-act race; {@link #removeReverseDep} may fail for a key that is returned here.
+ */
+ @ThreadSafe
+ Iterable<SkyKey> getReverseDeps();
/**
* Returns raw {@link SkyValue} stored in this entry, which may include metadata associated with
@@ -236,22 +267,22 @@
/**
* Should only be called if the entry is dirty. During the examination to see if the entry must be
- * re-evaluated, this method returns the next group of children to be checked. Callers should
- * have already called {@link #getDirtyState} and received a return value of
- * {@link DirtyState#CHECK_DEPENDENCIES} before calling this method -- any other
- * return value from {@link #getDirtyState} means that this method must not be called, since
- * whether or not the node needs to be rebuilt is already known.
+ * re-evaluated, this method returns the next group of children to be checked. Callers should have
+ * already called {@link #getDirtyState} and received a return value of {@link
+ * DirtyState#CHECK_DEPENDENCIES} before calling this method -- any other return value from {@link
+ * #getDirtyState} means that this method must not be called, since whether or not the node needs
+ * to be rebuilt is already known.
*
- * <p>Deps are returned in groups. The deps in each group were requested in parallel by the
- * {@code SkyFunction} last build, meaning independently of the values of any other deps in this
- * group (although possibly depending on deps in earlier groups). Thus the caller may check all
- * the deps in this group in parallel, since the deps in all previous groups are verified
- * unchanged. See {@link SkyFunction.Environment#getValues} for more on dependency groups.
+ * <p>Deps are returned in groups. The deps in each group were requested in parallel by the {@code
+ * SkyFunction} last build, meaning independently of the values of any other deps in this group
+ * (although possibly depending on deps in earlier groups). Thus the caller may check all the deps
+ * in this group in parallel, since the deps in all previous groups are verified unchanged. See
+ * {@link SkyFunction.Environment#getValues} for more on dependency groups.
*
* <p>The caller should register these as deps of this entry using {@link #addTemporaryDirectDeps}
* before checking them.
*
- * @see BuildingState#getNextDirtyDirectDeps()
+ * @see DirtyBuildingState#getNextDirtyDirectDeps()
*/
@ThreadSafe
Collection<SkyKey> getNextDirtyDirectDeps();
diff --git a/src/main/java/com/google/devtools/build/skyframe/ProcessableGraph.java b/src/main/java/com/google/devtools/build/skyframe/ProcessableGraph.java
index bc391a8..532a151 100644
--- a/src/main/java/com/google/devtools/build/skyframe/ProcessableGraph.java
+++ b/src/main/java/com/google/devtools/build/skyframe/ProcessableGraph.java
@@ -19,9 +19,8 @@
* A graph that is both Dirtiable (values can be deleted) and Evaluable (values can be added). All
* methods in this interface (as inherited from super-interfaces) should be thread-safe.
*
- * <p>This class is not intended for direct use, and is only exposed as public for use in
- * evaluation implementations outside of this package.
+ * <p>This class is not intended for direct use, and is only exposed as public for use in evaluation
+ * implementations outside of this package.
*/
@ThreadSafe
-public interface ProcessableGraph extends DirtiableGraph, EvaluableGraph {
-}
+public interface ProcessableGraph extends DeletableGraph, EvaluableGraph {}
diff --git a/src/main/java/com/google/devtools/build/skyframe/QueryableGraph.java b/src/main/java/com/google/devtools/build/skyframe/QueryableGraph.java
index ee09699..0419828 100644
--- a/src/main/java/com/google/devtools/build/skyframe/QueryableGraph.java
+++ b/src/main/java/com/google/devtools/build/skyframe/QueryableGraph.java
@@ -19,16 +19,11 @@
import javax.annotation.Nullable;
-/**
- * A graph that exposes its entries and structure, for use by classes that must traverse it.
- */
+/** A graph that exposes its entries and structure, for use by classes that must traverse it. */
@ThreadSafe
-public interface QueryableGraph extends ThinNodeQueryableGraph {
- /**
- * Returns the node with the given name, or {@code null} if the node does not exist.
- */
+public interface QueryableGraph {
+ /** Returns the node with the given name, or {@code null} if the node does not exist. */
@Nullable
- @Override
NodeEntry get(SkyKey key);
/**
@@ -36,6 +31,5 @@
* {@code keys}, {@code m.get(k).equals(e)} iff {@code get(k) == e} and {@code e != null}, and
* {@code !m.containsKey(k)} iff {@code get(k) == null}.
*/
- @Override
Map<SkyKey, NodeEntry> getBatch(Iterable<SkyKey> keys);
}
diff --git a/src/main/java/com/google/devtools/build/skyframe/ThinNodeEntry.java b/src/main/java/com/google/devtools/build/skyframe/ThinNodeEntry.java
index 1a5c075..f81355c 100644
--- a/src/main/java/com/google/devtools/build/skyframe/ThinNodeEntry.java
+++ b/src/main/java/com/google/devtools/build/skyframe/ThinNodeEntry.java
@@ -32,40 +32,6 @@
boolean isDone();
/**
- * Returns an immutable iterable of the direct deps of this node. This method may only be called
- * after the evaluation of this node is complete.
- *
- * <p>This method is not very efficient, but is only be called in limited circumstances --
- * when the node is about to be deleted, or when the node is expected to have no direct deps (in
- * which case the overhead is not so bad). It should not be called repeatedly for the same node,
- * since each call takes time proportional to the number of direct deps of the node.
- */
- @ThreadSafe
- Iterable<SkyKey> getDirectDeps();
-
- /**
- * Removes a reverse dependency.
- */
- @ThreadSafe
- void removeReverseDep(SkyKey reverseDep);
-
- /**
- * Removes a reverse dependency.
- *
- * <p>May only be called if this entry is not done (i.e. {@link #isDone} is false) and
- * {@param reverseDep} is present in {@link #getReverseDeps}
- */
- @ThreadSafe
- void removeInProgressReverseDep(SkyKey reverseDep);
-
- /**
- * Returns a copy of the set of reverse dependencies. Note that this introduces a potential
- * check-then-act race; {@link #removeReverseDep} may fail for a key that is returned here.
- */
- @ThreadSafe
- Iterable<SkyKey> getReverseDeps();
-
- /**
* Returns true if the entry is marked dirty, meaning that at least one of its transitive
* dependencies is marked changed.
*/
