Make support for interruptibility optional during dirtying
Reduces the amount of work needed when interruptibility isn't needed.
--
MOS_MIGRATED_REVID=108611609
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 cb2a8cc..49bcfa0 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 @@
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 @@
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 3854e94..2e012e9 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 @@
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 @@
}
@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 @@
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 @@
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 @@
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 @@
}
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 @@
"%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 @@
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 @@
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);
+ }
}
});
}
