diff options
author | Janak Ramakrishnan <janakr@google.com> | 2016-07-08 17:38:27 +0000 |
---|---|---|
committer | Kristina Chodorow <kchodorow@google.com> | 2016-07-11 09:39:22 +0000 |
commit | cc7712f0acff385046d76b9012eeb342452d93ac (patch) | |
tree | f5d532b527aab6ece71fa4502f898db75dd0aea2 /src/main/java/com/google/devtools/build | |
parent | a59a8b83cd57720fd9579378a96965c6b56dccc1 (diff) |
Refactor QueryableGraph and ThinNodeQueryableGraph to be independent interfaces, in preparation for further changes.
--
MOS_MIGRATED_REVID=126924789
Diffstat (limited to 'src/main/java/com/google/devtools/build')
13 files changed, 98 insertions, 116 deletions
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 8e9ddb62b7..d714924e29 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 @@ public abstract class DelegatingNodeEntry implements NodeEntry { @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 index fe7e88bdb8..d091f083d6 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 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; * 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 bb6d2b2685..b7c1ee9545 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 @@ public final class EagerInvalidator { * 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 @@ public final class EagerInvalidator { @Nullable static DeletingNodeVisitor createDeletingVisitorIfNeeded( - DirtiableGraph graph, + InMemoryGraph graph, Iterable<SkyKey> diff, EvaluationProgressReceiver invalidationReceiver, InvalidationState state, @@ -70,7 +75,7 @@ public final class EagerInvalidator { @Nullable static DirtyingNodeVisitor createInvalidatingVisitorIfNeeded( - ThinNodeQueryableGraph graph, + InvalidatableGraph graph, Iterable<SkyKey> diff, EvaluationProgressReceiver invalidationReceiver, InvalidationState state, @@ -84,7 +89,7 @@ public final class EagerInvalidator { @Nullable private static DirtyingNodeVisitor createInvalidatingVisitorIfNeeded( - ThinNodeQueryableGraph graph, + InvalidatableGraph graph, Iterable<SkyKey> diff, EvaluationProgressReceiver invalidationReceiver, InvalidationState state, @@ -110,7 +115,7 @@ public final class EagerInvalidator { * 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 @@ public final class EagerInvalidator { * the provided {@link ForkJoinPool}. */ public static void invalidate( - ThinNodeQueryableGraph graph, + InvalidatableGraph graph, Iterable<SkyKey> diff, EvaluationProgressReceiver invalidationReceiver, InvalidationState state, @@ -154,11 +159,9 @@ public final class EagerInvalidator { } } - /** - * 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 7527146b30..b3d8bd0ac2 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 @@ import java.util.Map; * 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 02735c0e8e..5d508a9eb1 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 @@ package com.google.devtools.build.skyframe; 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 a5e22221e6..5d853b7aff 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 @@ public final class InMemoryMemoizingEvaluator implements MemoizingEvaluator { } @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 index eeb3ef5867..243740705a 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 com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; 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 6eec236b4e..1fe90d193e 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 @@ import javax.annotation.Nullable; * * <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 @@ public abstract class InvalidatingNodeVisitor<TGraph extends ThinNodeQueryableGr } /** 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 @@ public abstract class InvalidatingNodeVisitor<TGraph extends ThinNodeQueryableGr } /** 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 @@ public abstract class InvalidatingNodeVisitor<TGraph extends ThinNodeQueryableGr private final boolean supportInterruptions; protected DirtyingNodeVisitor( - ThinNodeQueryableGraph graph, + InvalidatableGraph graph, EvaluationProgressReceiver invalidationReceiver, InvalidationState state, DirtyKeyTracker dirtyKeyTracker, @@ -365,7 +365,7 @@ public abstract class InvalidatingNodeVisitor<TGraph extends ThinNodeQueryableGr * 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 269ea0927b..4d5d21c5de 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 @@ public interface MemoizingEvaluator { /** * 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 7b38324110..8b7b09193d 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 @@ public interface NodeEntry extends ThinNodeEntry { @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 @@ public interface NodeEntry extends ThinNodeEntry { /** * 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 bc391a852f..532a151ef8 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 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; * 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 ee096996ac..04198286d8 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 java.util.Map; 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 @@ public interface QueryableGraph extends ThinNodeQueryableGraph { * {@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 1a5c075073..f81355c271 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 @@ public interface ThinNodeEntry { 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. */ |