diff options
author | Googler <noreply@google.com> | 2016-09-13 14:51:46 +0000 |
---|---|---|
committer | Dmitry Lomov <dslomov@google.com> | 2016-09-14 09:34:04 +0000 |
commit | 7a0b5187052859c3c7c204dcc2067805bd69cb3d (patch) | |
tree | 5a719f4c8e311503a6efdd5714ceefeab237b73d /src/main/java/com/google/devtools/build | |
parent | 47549f90412387ee69383d4ec7fa299e551f3040 (diff) |
Allow reverse dependency lookups on Skyframe graph to throw InteruptedException
--
MOS_MIGRATED_REVID=132999234
Diffstat (limited to 'src/main/java/com/google/devtools/build')
4 files changed, 43 insertions, 17 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 6bf77ec4e0..47a9e616dd 100644 --- a/src/main/java/com/google/devtools/build/skyframe/DelegatingNodeEntry.java +++ b/src/main/java/com/google/devtools/build/skyframe/DelegatingNodeEntry.java @@ -64,12 +64,14 @@ public abstract class DelegatingNodeEntry implements NodeEntry { } @Override - public DependencyState addReverseDepAndCheckIfDone(@Nullable SkyKey reverseDep) { + public DependencyState addReverseDepAndCheckIfDone(@Nullable SkyKey reverseDep) + throws InterruptedException { return getDelegate().addReverseDepAndCheckIfDone(reverseDep); } @Override - public DependencyState checkIfDoneForDirtyReverseDep(SkyKey reverseDep) { + public DependencyState checkIfDoneForDirtyReverseDep(SkyKey reverseDep) + throws InterruptedException { return getDelegate().checkIfDoneForDirtyReverseDep(reverseDep); } @@ -159,7 +161,7 @@ public abstract class DelegatingNodeEntry implements NodeEntry { } @Override - public void removeReverseDep(SkyKey reverseDep) { + public void removeReverseDep(SkyKey reverseDep) throws InterruptedException { getDelegate().removeReverseDep(reverseDep); } @@ -169,7 +171,7 @@ public abstract class DelegatingNodeEntry implements NodeEntry { } @Override - public Iterable<SkyKey> getReverseDeps() { + public Iterable<SkyKey> getReverseDeps() throws InterruptedException { return getDelegate().getReverseDeps(); } 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 096fc2d49b..8af633d2c4 100644 --- a/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java +++ b/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java @@ -282,7 +282,16 @@ public abstract class InvalidatingNodeVisitor<TGraph extends QueryableGraph> { if (traverseGraph) { // Propagate deletion upwards. - visit(entry.getReverseDeps(), InvalidationType.DELETED); + try { + visit(entry.getReverseDeps(), InvalidationType.DELETED); + } catch (InterruptedException e) { + throw new IllegalStateException( + "Deletion cannot happen on a graph that may have blocking operations: " + + key + + ", " + + entry, + e); + } // Unregister this node as an rdep from its direct deps, since reverse dep // edges cannot point to non-existent nodes. To know whether the child has this @@ -318,7 +327,17 @@ public abstract class InvalidatingNodeVisitor<TGraph extends QueryableGraph> { NodeEntry dep = directDepEntry.getValue(); if (dep != null) { if (dep.isDone() || !signalingDeps.contains(directDepEntry.getKey())) { - dep.removeReverseDep(key); + try { + dep.removeReverseDep(key); + } catch (InterruptedException e) { + throw new IllegalStateException( + "Deletion cannot happen on a graph that may have blocking " + + "operations: " + + key + + ", " + + entry, + e); + } } else { // This step is not strictly necessary, since all in-progress nodes are // deleted during graph cleaning, which happens in a single 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 82df6fa2ff..ee75ac3cf1 100644 --- a/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java +++ b/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java @@ -98,7 +98,7 @@ public interface NodeEntry extends ThinNodeEntry { /** Removes a reverse dependency. */ @ThreadSafe - void removeReverseDep(SkyKey reverseDep); + void removeReverseDep(SkyKey reverseDep) throws InterruptedException; /** * Removes a reverse dependency. @@ -114,7 +114,7 @@ public interface NodeEntry extends ThinNodeEntry { * check-then-act race; {@link #removeReverseDep} may fail for a key that is returned here. */ @ThreadSafe - Iterable<SkyKey> getReverseDeps(); + Iterable<SkyKey> getReverseDeps() throws InterruptedException; /** * Returns raw {@link SkyValue} stored in this entry, which may include metadata associated with @@ -166,9 +166,9 @@ public interface NodeEntry extends ThinNodeEntry { /** * Queries if the node is done and adds the given key as a reverse dependency. The return code - * indicates whether a) the node is done, b) the reverse dependency is the first one, so the - * node needs to be scheduled, or c) the reverse dependency was added, and the node does not - * need to be scheduled. + * indicates whether a) the node is done, b) the reverse dependency is the first one, so the node + * needs to be scheduled, or c) the reverse dependency was added, and the node does not need to be + * scheduled. * * <p>This method <b>must</b> be called before any processing of the entry. This encourages * callers to check that the entry is ready to be processed. @@ -179,14 +179,15 @@ public interface NodeEntry extends ThinNodeEntry { * However, a may complete first, in which case b has to re-schedule itself. Also see {@link * #setValue}. * - * <p>If the parameter is {@code null}, then no reverse dependency is added, but we still check - * if the node needs to be scheduled. + * <p>If the parameter is {@code null}, then no reverse dependency is added, but we still check if + * the node needs to be scheduled. * * <p>If {@code reverseDep} is a rebuilding dirty entry that was already a reverse dep of this * entry, then {@link #checkIfDoneForDirtyReverseDep} must be called instead. */ @ThreadSafe - DependencyState addReverseDepAndCheckIfDone(@Nullable SkyKey reverseDep); + DependencyState addReverseDepAndCheckIfDone(@Nullable SkyKey reverseDep) + throws InterruptedException; /** * Similar to {@link #addReverseDepAndCheckIfDone}, except that {@param reverseDep} must already @@ -196,7 +197,7 @@ public interface NodeEntry extends ThinNodeEntry { * node for evaluation if needed. */ @ThreadSafe - DependencyState checkIfDoneForDirtyReverseDep(SkyKey reverseDep); + DependencyState checkIfDoneForDirtyReverseDep(SkyKey reverseDep) throws InterruptedException; /** * Tell this node that one of its dependencies is now done. Callers must check the return value, diff --git a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java index aee81ead3b..8cfe1f6e59 100644 --- a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java +++ b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java @@ -207,7 +207,8 @@ public final class ParallelEvaluator implements Evaluator { NodeEntry entry, SkyKey child, NodeEntry childEntry, - boolean depAlreadyExists) { + boolean depAlreadyExists) + throws InterruptedException { Preconditions.checkState(!entry.isDone(), "%s %s", skyKey, entry); DependencyState dependencyState = depAlreadyExists @@ -595,13 +596,16 @@ public final class ParallelEvaluator implements Evaluator { * its requested deps are done. However, that means we're assuming the SkyFunction would throw * that same error if all of its requested deps were done. Unfortunately, there is no way to * enforce that condition. + * + * @throws InterruptedException */ private static void registerNewlyDiscoveredDepsForDoneEntry( SkyKey skyKey, NodeEntry entry, Map<SkyKey, ? extends NodeEntry> newlyRequestedDepMap, Set<SkyKey> oldDeps, - SkyFunctionEnvironment env) { + SkyFunctionEnvironment env) + throws InterruptedException { Set<SkyKey> unfinishedDeps = new HashSet<>(); for (SkyKey dep : env.getNewlyRequestedDeps()) { if (!isDoneForBuild(newlyRequestedDepMap.get(dep))) { |