Clean up creation of a SkyFunction#Restart.

Make `Restart` a class since there is only one acceptable implementation. Require that the rewind graph at minimum contains the key to restart - empty rewind graphs were only used in now deleted tests.

Provide a convenience method that creates a `MutableGraph` with basic requirements met.

PiperOrigin-RevId: 560151638
Change-Id: Ieadb9480e9b4c2262e3e2f2c51d96d48412bdb0d

src/main/java/com/google/devtools/build/lib/skyframe/rewinding/ActionRewindStrategy.java

@@ -34,7 +34,6 @@
 import com.google.common.collect.MultimapBuilder;
 import com.google.common.flogger.GoogleLogger;
 import com.google.common.graph.EndpointPair;
-import com.google.common.graph.GraphBuilder;
 import com.google.common.graph.ImmutableGraph;
 import com.google.common.graph.MutableGraph;
 import com.google.common.graph.Traverser;
@@ -122,8 +121,7 @@ public RewindPlan getRewindPlan(
 
     // This graph tracks which Skyframe nodes must be restarted and the dependency relationships
     // between them.
-    MutableGraph<SkyKey> rewindGraph = GraphBuilder.directed().allowsSelfLoops(false).build();
-    rewindGraph.addNode(actionLookupData);
+    MutableGraph<SkyKey> rewindGraph = Restart.newRewindGraphFor(actionLookupData);
 
     // SkyframeActionExecutor must re-execute the actions being restarted, so we must tell it to
     // evict its cached results for those actions. This collection tracks those actions (aside from
@@ -178,8 +176,7 @@ public RewindPlan getRewindPlan(
             lostInputRecordsCount,
             lostInputRecordsThisAction.subList(
                 0, Math.min(lostInputRecordsCount, MAX_LOST_INPUTS_RECORDED))));
-    return new RewindPlan(
-        Restart.selfAnd(ImmutableGraph.copyOf(rewindGraph)), additionalActionsToRestart.build());
+    return new RewindPlan(Restart.of(rewindGraph), additionalActionsToRestart.build());
   }
 
   /**

src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java

@@ -13,7 +13,6 @@
 // limitations under the License.
 package com.google.devtools.build.skyframe;
 
-import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
 
@@ -862,18 +861,14 @@ protected void replay(ValueWithMetadata valueWithMetadata) {
   private void dirtyRewindGraphAndResetEntry(SkyKey key, NodeEntry entry, Restart restart)
       throws InterruptedException {
     ImmutableGraph<SkyKey> rewindGraph = restart.rewindGraph();
-    if (rewindGraph.nodes().isEmpty()) {
-      resetEntry(key, entry);
-      return;
-    }
-    checkArgument(
+    checkState(
         rewindGraph.nodes().contains(key),
-        "rewindGraph must contain the key for the failed evaluation if it's not empty. key: %s, "
-            + "rewindGraph: %s",
+        "Rewind graph missing evaluating key %s: %s",
         key,
         rewindGraph);
 
-    ImmutableList.Builder<SkyKey> builder = ImmutableList.builder();
+    ImmutableList.Builder<SkyKey> builder =
+        ImmutableList.builderWithExpectedSize(rewindGraph.nodes().size() - 1);
     for (SkyKey k : Traverser.forGraph(rewindGraph).depthFirstPostOrder(key)) {
       if (!k.equals(key)) {
         builder.add(k);

src/main/java/com/google/devtools/build/skyframe/SkyFunction.java

@@ -13,11 +13,13 @@
 // limitations under the License.
 package com.google.devtools.build.skyframe;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
 
-import com.google.common.base.Preconditions;
+import com.google.common.graph.Graph;
 import com.google.common.graph.GraphBuilder;
 import com.google.common.graph.ImmutableGraph;
+import com.google.common.graph.MutableGraph;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
@@ -121,21 +123,36 @@ default Version getMaxTransitiveSourceVersionToInjectForNonHermeticFunction(
    * true. If restarting is not permitted, {@link #compute} should throw an appropriate {@link
    * SkyFunctionException}.
    */
-  interface Restart extends SkyValue {
-    ImmutableGraph<SkyKey> EMPTY_SKYKEY_GRAPH =
-        ImmutableGraph.copyOf(GraphBuilder.directed().allowsSelfLoops(false).build());
-
-    Restart SELF = () -> EMPTY_SKYKEY_GRAPH;
-
-    static Restart selfAnd(ImmutableGraph<SkyKey> rewindGraph) {
-      Preconditions.checkArgument(
-          rewindGraph.isDirected(), "rewindGraph undirected: %s", rewindGraph);
-      Preconditions.checkArgument(
-          !rewindGraph.allowsSelfLoops(), "rewindGraph allows self loops: %s", rewindGraph);
-      return () -> rewindGraph;
+  final class Restart implements SkyValue {
+
+    /**
+     * Convenience method that creates a {@link MutableGraph} that fulfills the basic requirements
+     * of a {@link Restart}.
+     *
+     * <p>Additional edges may be added to the graph before passing to {@link #of}.
+     */
+    public static MutableGraph<SkyKey> newRewindGraphFor(SkyKey keyToRestart) {
+      MutableGraph<SkyKey> rewindGraph = GraphBuilder.directed().allowsSelfLoops(false).build();
+      rewindGraph.addNode(keyToRestart);
+      return rewindGraph;
+    }
+
+    public static Restart of(Graph<SkyKey> rewindGraph) {
+      checkArgument(rewindGraph.isDirected(), "Undirected: %s", rewindGraph);
+      checkArgument(!rewindGraph.allowsSelfLoops(), "Allows self loops: %s", rewindGraph);
+      checkArgument(!rewindGraph.nodes().isEmpty(), "Rewind graph must include key to restart");
+      return new Restart(ImmutableGraph.copyOf(rewindGraph));
     }
 
-    ImmutableGraph<SkyKey> rewindGraph();
+    private final ImmutableGraph<SkyKey> rewindGraph;
+
+    private Restart(ImmutableGraph<SkyKey> rewindGraph) {
+      this.rewindGraph = rewindGraph;
+    }
+
+    ImmutableGraph<SkyKey> rewindGraph() {
+      return rewindGraph;
+    }
   }
 
   /**