Create a dedicated graph lookup reason for rewinding and add a wildcard to createIfAbsentBatch to match getBatch.

justinhorvitz · copybara-github · commit a0a0d09debd0 · 2022-05-31T16:58:38.000-07:00
PiperOrigin-RevId: 452172428
Change-Id: I2ed3fbd0a92568b231aa06471ee0be1cb574f987
diff --git a/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java
@@ -989,7 +989,7 @@ private boolean maybeHandleRestart(SkyKey key, NodeEntry entry, SkyValue returne
     }
 
     Map<SkyKey, ? extends NodeEntry> additionalNodesToRestart =
-        evaluatorContext.getBatchValues(key, Reason.INVALIDATION, additionalKeysToRestart);
+        evaluatorContext.getBatchValues(key, Reason.REWINDING, additionalKeysToRestart);
 
     ArrayList<SkyKey> missingNodes = null;
     for (SkyKey keyToRestart : additionalKeysToRestart) {
diff --git a/src/main/java/com/google/devtools/build/skyframe/InMemoryGraph.java b/src/main/java/com/google/devtools/build/skyframe/InMemoryGraph.java
@@ -39,7 +39,7 @@ static InMemoryGraph createEdgeless() {
 
   @Override
   Map<SkyKey, ? extends NodeEntry> createIfAbsentBatch(
-      @Nullable SkyKey requestor, Reason reason, Iterable<SkyKey> keys);
+      @Nullable SkyKey requestor, Reason reason, Iterable<? extends SkyKey> keys);
 
   @Nullable
   @Override
diff --git a/src/main/java/com/google/devtools/build/skyframe/InMemoryGraphImpl.java b/src/main/java/com/google/devtools/build/skyframe/InMemoryGraphImpl.java
@@ -82,7 +82,7 @@ protected InMemoryNodeEntry newNodeEntry(SkyKey key) {
 
   @Override
   public Map<SkyKey, NodeEntry> createIfAbsentBatch(
-      @Nullable SkyKey requestor, Reason reason, Iterable<SkyKey> keys) {
+      @Nullable SkyKey requestor, Reason reason, Iterable<? extends SkyKey> keys) {
     Map<SkyKey, NodeEntry> result = CompactHashMap.createWithExpectedSize(Iterables.size(keys));
     for (SkyKey key : keys) {
       result.put(key, nodeMap.computeIfAbsent(key, newNodeEntryFunction));
diff --git a/src/main/java/com/google/devtools/build/skyframe/ProcessableGraph.java b/src/main/java/com/google/devtools/build/skyframe/ProcessableGraph.java
@@ -45,7 +45,8 @@ public interface ProcessableGraph extends QueryableGraph {
    * @param reason the reason the nodes are being requested.
    */
   Map<SkyKey, ? extends NodeEntry> createIfAbsentBatch(
-      @Nullable SkyKey requestor, Reason reason, Iterable<SkyKey> keys) throws InterruptedException;
+      @Nullable SkyKey requestor, Reason reason, Iterable<? extends SkyKey> keys)
+      throws InterruptedException;
 
   /**
    * Like {@link QueryableGraph#getBatchAsync}, except it creates a new node for each key not
diff --git a/src/main/java/com/google/devtools/build/skyframe/QueryableGraph.java b/src/main/java/com/google/devtools/build/skyframe/QueryableGraph.java
@@ -53,7 +53,7 @@ public interface QueryableGraph {
    */
   Map<SkyKey, ? extends NodeEntry> getBatch(
       @Nullable SkyKey requestor, Reason reason, Iterable<? extends SkyKey> keys)
-          throws InterruptedException;
+      throws InterruptedException;
 
   /**
    * A version of {@link #getBatch} that returns an {@link InterruptibleSupplier} to possibly
@@ -153,6 +153,12 @@ enum Reason {
     /** The node is being looked up merely to see if it is done or not. */
     DONE_CHECKING,
 
+    /**
+     * The node is being looked up so that it can be {@linkplain
+     * ThinNodeEntry.DirtyType#FORCE_REBUILD force rebuilt} by rewinding.
+     */
+    REWINDING,
+
     /**
      * The node is being looked up to service {@link WalkableGraph#getValue},
      * {@link WalkableGraph#getException}, {@link WalkableGraph#getMissingAndExceptions}, or
diff --git a/src/test/java/com/google/devtools/build/skyframe/DeterministicHelper.java b/src/test/java/com/google/devtools/build/skyframe/DeterministicHelper.java
@@ -115,7 +115,7 @@ public void remove(SkyKey key) {
 
     @Override
     public Map<SkyKey, ? extends NodeEntry> createIfAbsentBatch(
-        @Nullable SkyKey requestor, Reason reason, Iterable<SkyKey> keys)
+        @Nullable SkyKey requestor, Reason reason, Iterable<? extends SkyKey> keys)
         throws InterruptedException {
       return makeDeterministic(super.createIfAbsentBatch(requestor, reason, keys));
     }
diff --git a/src/test/java/com/google/devtools/build/skyframe/DeterministicInMemoryGraph.java b/src/test/java/com/google/devtools/build/skyframe/DeterministicInMemoryGraph.java
@@ -30,7 +30,7 @@ class DeterministicInMemoryGraph extends DeterministicHelper.DeterministicProces
 
   @Override
   public Map<SkyKey, ? extends NodeEntry> createIfAbsentBatch(
-      @Nullable SkyKey requestor, Reason reason, Iterable<SkyKey> keys) {
+      @Nullable SkyKey requestor, Reason reason, Iterable<? extends SkyKey> keys) {
     try {
       return super.createIfAbsentBatch(requestor, reason, keys);
     } catch (InterruptedException e) {
diff --git a/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java b/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java
@@ -17,7 +17,6 @@
 import com.google.common.base.Joiner;
 import com.google.common.base.MoreObjects;
 import com.google.common.collect.Maps;
-import com.google.common.collect.Maps.EntryTransformer;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
 import com.google.devtools.build.lib.util.GroupedList;
 import com.google.devtools.build.lib.util.GroupedList.GroupedListHelper;
@@ -55,15 +54,6 @@ public ProcessableGraph transform(ProcessableGraph graph) {
 
   protected final Listener graphListener;
 
-  protected final EntryTransformer<SkyKey, ThinNodeEntry, NodeEntry> wrapEntry =
-      new EntryTransformer<SkyKey, ThinNodeEntry, NodeEntry>() {
-        @Nullable
-        @Override
-        public NotifyingNodeEntry transformEntry(SkyKey key, @Nullable ThinNodeEntry nodeEntry) {
-          return wrapEntry(key, nodeEntry);
-        }
-      };
-
   NotifyingHelper(Listener graphListener) {
     this.graphListener = new ErrorRecordingDelegatingListener(graphListener);
   }
@@ -96,8 +86,7 @@ static class NotifyingQueryableGraph implements QueryableGraph {
         notifyingHelper.graphListener.accept(key, EventType.GET_BATCH, Order.BEFORE, reason);
       }
       return Maps.transformEntries(
-          delegate.getBatch(requestor, reason, keys),
-          notifyingHelper.wrapEntry);
+          delegate.getBatch(requestor, reason, keys), notifyingHelper::wrapEntry);
     }
 
     @Nullable
@@ -129,14 +118,13 @@ public void remove(SkyKey key) {
 
     @Override
     public Map<SkyKey, ? extends NodeEntry> createIfAbsentBatch(
-        @Nullable SkyKey requestor, Reason reason, Iterable<SkyKey> keys)
+        @Nullable SkyKey requestor, Reason reason, Iterable<? extends SkyKey> keys)
         throws InterruptedException {
       for (SkyKey key : keys) {
         notifyingHelper.graphListener.accept(key, EventType.CREATE_IF_ABSENT, Order.BEFORE, null);
       }
       return Maps.transformEntries(
-          delegate.createIfAbsentBatch(requestor, reason, keys),
-          notifyingHelper.wrapEntry);
+          delegate.createIfAbsentBatch(requestor, reason, keys), notifyingHelper::wrapEntry);
     }
 
     @Override
diff --git a/src/test/java/com/google/devtools/build/skyframe/NotifyingInMemoryGraph.java b/src/test/java/com/google/devtools/build/skyframe/NotifyingInMemoryGraph.java
@@ -26,7 +26,7 @@ class NotifyingInMemoryGraph extends NotifyingHelper.NotifyingProcessableGraph
 
   @Override
   public Map<SkyKey, ? extends NodeEntry> createIfAbsentBatch(
-      @Nullable SkyKey requestor, Reason reason, Iterable<SkyKey> keys) {
+      @Nullable SkyKey requestor, Reason reason, Iterable<? extends SkyKey> keys) {
     try {
       return super.createIfAbsentBatch(requestor, reason, keys);
     } catch (InterruptedException e) {

Original file line number	Diff line number	Diff line change
`@@ -989,7 +989,7 @@ private boolean maybeHandleRestart(SkyKey key, NodeEntry entry, SkyValue returne`
`989`	`989`	`}`
`990`	`990`
`991`	`991`	`Map<SkyKey, ? extends NodeEntry> additionalNodesToRestart =`
`992`		`- evaluatorContext.getBatchValues(key, Reason.INVALIDATION, additionalKeysToRestart);`
	`992`	`+ evaluatorContext.getBatchValues(key, Reason.REWINDING, additionalKeysToRestart);`
`993`	`993`
`994`	`994`	`ArrayList<SkyKey> missingNodes = null;`
`995`	`995`	`for (SkyKey keyToRestart : additionalKeysToRestart) {`
Original file line number	Diff line number	Diff line change
`@@ -115,7 +115,7 @@ public void remove(SkyKey key) {`
`115`	`115`
`116`	`116`	`@Override`
`117`	`117`	`public Map<SkyKey, ? extends NodeEntry> createIfAbsentBatch(`
`118`		`- @Nullable SkyKey requestor, Reason reason, Iterable<SkyKey> keys)`
	`118`	`+ @Nullable SkyKey requestor, Reason reason, Iterable<? extends SkyKey> keys)`
`119`	`119`	`throws InterruptedException {`
`120`	`120`	`return makeDeterministic(super.createIfAbsentBatch(requestor, reason, keys));`
`121`	`121`	`}`