diff options
author | 2015-11-24 15:45:40 +0000 | |
---|---|---|
committer | 2015-11-25 10:00:13 +0000 | |
commit | c6ed6136c563f9836670ff740388e7b6fcdf8c83 (patch) | |
tree | 41d1cab3d0598ff72a7ad82fa0ca358cd5849a5a /src/main/java/com/google | |
parent | 5b3290016a29fa2cf01a68cd0025d8982a432882 (diff) |
Make support for interruptibility optional during dirtying
Reduces the amount of work needed when interruptibility isn't needed.
--
MOS_MIGRATED_REVID=108611609
Diffstat (limited to 'src/main/java/com/google')
-rw-r--r-- | src/main/java/com/google/devtools/build/skyframe/EagerInvalidator.java | 21 | ||||
-rw-r--r-- | src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java | 46 |
2 files changed, 56 insertions, 11 deletions
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 cb2a8ccf66..49bcfa0165 100644 --- a/src/main/java/com/google/devtools/build/skyframe/EagerInvalidator.java +++ b/src/main/java/com/google/devtools/build/skyframe/EagerInvalidator.java @@ -88,12 +88,18 @@ public final class EagerInvalidator { EvaluationProgressReceiver invalidationReceiver, InvalidationState state, DirtyKeyTracker dirtyKeyTracker, - ForkJoinPool forkJoinPool) { + ForkJoinPool forkJoinPool, + boolean supportInterruptions) { state.update(diff); return state.isEmpty() ? null : new DirtyingNodeVisitor( - graph, invalidationReceiver, state, dirtyKeyTracker, forkJoinPool); + graph, + invalidationReceiver, + state, + dirtyKeyTracker, + forkJoinPool, + supportInterruptions); } /** @@ -126,11 +132,18 @@ public final class EagerInvalidator { EvaluationProgressReceiver invalidationReceiver, InvalidationState state, DirtyKeyTracker dirtyKeyTracker, - ForkJoinPool forkJoinPool) + ForkJoinPool forkJoinPool, + boolean supportInterruptions) throws InterruptedException { DirtyingNodeVisitor visitor = createInvalidatingVisitorIfNeeded( - graph, diff, invalidationReceiver, state, dirtyKeyTracker, forkJoinPool); + graph, + diff, + invalidationReceiver, + state, + dirtyKeyTracker, + forkJoinPool, + supportInterruptions); if (visitor != null) { visitor.run(); } 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 3854e94fc3..2e012e90be 100644 --- a/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java +++ b/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java @@ -148,10 +148,14 @@ public abstract class InvalidatingNodeVisitor<TGraph extends ThinNodeQueryableGr visit(ImmutableList.of(visitData.first), visitData.second, !MUST_EXIST); } executor.awaitQuiescence(/*interruptWorkers=*/ true); - Preconditions.checkState(pendingVisitations.isEmpty(), + + // Note: implementations that do not support interruption also do not update pendingVisitations. + Preconditions.checkState(!getSupportInterruptions() || pendingVisitations.isEmpty(), "All dirty nodes should have been processed: %s", pendingVisitations); } + protected abstract boolean getSupportInterruptions(); + @VisibleForTesting public CountDownLatch getInterruptionLatchForTestingOnly() { return executor.getInterruptionLatchForTestingOnly(); @@ -250,6 +254,11 @@ public abstract class InvalidatingNodeVisitor<TGraph extends ThinNodeQueryableGr } @Override + protected boolean getSupportInterruptions() { + return true; + } + + @Override public void visit(Iterable<SkyKey> keys, InvalidationType invalidationType, boolean mustExist) { Preconditions.checkState(invalidationType == InvalidationType.DELETED, keys); Builder<SkyKey> unvisitedKeysBuilder = ImmutableList.builder(); @@ -335,6 +344,7 @@ public abstract class InvalidatingNodeVisitor<TGraph extends ThinNodeQueryableGr Collections.newSetFromMap( new ConcurrentHashMap<Pair<SkyKey, InvalidationType>, Boolean>( EXPECTED_VISITED_SET_SIZE, .75f, DEFAULT_THREAD_COUNT)); + private final boolean supportInterruptions; protected DirtyingNodeVisitor( ThinNodeQueryableGraph graph, @@ -343,15 +353,22 @@ public abstract class InvalidatingNodeVisitor<TGraph extends ThinNodeQueryableGr DirtyKeyTracker dirtyKeyTracker, Function<ExecutorParams, ? extends ExecutorService> executorFactory) { super(graph, invalidationReceiver, state, dirtyKeyTracker, executorFactory); + this.supportInterruptions = true; } + /** + * Use cases that do not require support for interruptibility can avoid unnecessary work by + * passing {@code false} for {@param supportInterruptions}. + */ protected DirtyingNodeVisitor( ThinNodeQueryableGraph graph, EvaluationProgressReceiver invalidationReceiver, InvalidationState state, DirtyKeyTracker dirtyKeyTracker, - ForkJoinPool forkJoinPool) { + ForkJoinPool forkJoinPool, + boolean supportInterruptions) { super(graph, invalidationReceiver, state, dirtyKeyTracker, forkJoinPool); + this.supportInterruptions = supportInterruptions; } @Override @@ -359,6 +376,11 @@ public abstract class InvalidatingNodeVisitor<TGraph extends ThinNodeQueryableGr return visited.size(); } + @Override + protected boolean getSupportInterruptions() { + return supportInterruptions; + } + /** * Queues a task to dirty the nodes named by {@param keys}. May be called from multiple threads. * It is possible that the same node is enqueued many times. However, we require that a node @@ -401,7 +423,9 @@ public abstract class InvalidatingNodeVisitor<TGraph extends ThinNodeQueryableGr } List<SkyKey> keysToGet = Lists.transform(invalidationPairs, Pair.<SkyKey, InvalidationType>firstFunction()); - pendingVisitations.addAll(invalidationPairs); + if (supportInterruptions) { + pendingVisitations.addAll(invalidationPairs); + } final Map<SkyKey, ? extends ThinNodeEntry> entries = graph.getBatch(keysToGet); for (final Pair<SkyKey, InvalidationType> invalidationPair : invalidationPairs) { executor.execute( @@ -417,14 +441,18 @@ public abstract class InvalidatingNodeVisitor<TGraph extends ThinNodeQueryableGr "%s does not exist in the graph but was enqueued for dirtying by another " + "node", key); - pendingVisitations.remove(invalidationPair); + if (supportInterruptions) { + pendingVisitations.remove(invalidationPair); + } return; } if (entry.isChanged() || (!isChanged && entry.isDirty())) { // If this node is already marked changed, or we are only marking this node // dirty, and it already is, move along. - pendingVisitations.remove(invalidationPair); + if (supportInterruptions) { + pendingVisitations.remove(invalidationPair); + } return; } @@ -435,7 +463,9 @@ public abstract class InvalidatingNodeVisitor<TGraph extends ThinNodeQueryableGr MarkedDirtyResult markedDirtyResult = entry.markDirty(isChanged); if (markedDirtyResult == null) { // Another thread has already dirtied this node. Don't do anything in this thread. - pendingVisitations.remove(invalidationPair); + if (supportInterruptions) { + pendingVisitations.remove(invalidationPair); + } return; } // Propagate dirtiness upwards and mark this node dirty/changed. Reverse deps should @@ -446,7 +476,9 @@ public abstract class InvalidatingNodeVisitor<TGraph extends ThinNodeQueryableGr informInvalidationReceiver(key, EvaluationProgressReceiver.InvalidationState.DIRTY); dirtyKeyTracker.dirty(key); // Remove the node from the set as the last operation. - pendingVisitations.remove(invalidationPair); + if (supportInterruptions) { + pendingVisitations.remove(invalidationPair); + } } }); } |