From 414602731b299e0f20fc2d5dab774ba2b70bb58f Mon Sep 17 00:00:00 2001
From: Googler <jhorvitz@google.com>
Date: Thu, 16 Nov 2023 13:13:32 -0800
Subject: [PATCH] Some improvements to `analyzeDepsDoneness`.

* Document what the parameters are and when it is called.
* Tighten the type of `deps` from `Collection` to `List`.

PiperOrigin-RevId: 583141189
Change-Id: I672aee8a71fc9df27532f2eb74934fe3db0b6371
---
 .../build/skyframe/InMemoryGraphImpl.java         |  3 ++-
 .../devtools/build/skyframe/ProcessableGraph.java | 15 ++++++++++-----
 .../devtools/build/skyframe/NotifyingHelper.java  |  3 +--
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/skyframe/InMemoryGraphImpl.java b/src/main/java/com/google/devtools/build/skyframe/InMemoryGraphImpl.java
index ffdc4f6862ed96..7c049dae7d508d 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InMemoryGraphImpl.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryGraphImpl.java
@@ -35,6 +35,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.concurrent.ConcurrentHashMap;
@@ -201,7 +202,7 @@ private InMemoryNodeEntry createIfAbsent(SkyKey skyKey) {
   }
 
   @Override
-  public DepsReport analyzeDepsDoneness(SkyKey parent, Collection<SkyKey> deps) {
+  public DepsReport analyzeDepsDoneness(SkyKey parent, List<SkyKey> deps) {
     return DepsReport.NO_INFORMATION;
   }
 
diff --git a/src/main/java/com/google/devtools/build/skyframe/ProcessableGraph.java b/src/main/java/com/google/devtools/build/skyframe/ProcessableGraph.java
index b0d3c8b1b217af..6801316515cb60 100644
--- a/src/main/java/com/google/devtools/build/skyframe/ProcessableGraph.java
+++ b/src/main/java/com/google/devtools/build/skyframe/ProcessableGraph.java
@@ -17,7 +17,7 @@
 import com.google.devtools.build.lib.supplier.InterruptibleSupplier;
 import com.google.devtools.build.lib.supplier.MemoizingInterruptibleSupplier;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
-import java.util.Collection;
+import java.util.List;
 import javax.annotation.Nullable;
 
 /**
@@ -77,9 +77,14 @@ default InterruptibleSupplier<NodeBatch> createIfAbsentBatchAsync(
    * have not been recomputed since the last computation of {@code parent}. When determining if
    * {@code parent} needs to be re-evaluated, this may be used to avoid unnecessary graph accesses.
    *
-   * <p>Returns deps that may have new values since the node of {@code parent} was last computed,
-   * and therefore which may force re-evaluation of the node of {@code parent}.
+   * <p>If this graph partakes in the optional optimization, returns deps that may have new values
+   * since the node of {@code parent} was last computed, and therefore which may force re-evaluation
+   * of the node of {@code parent}. Otherwise, returns {@link DepsReport#NO_INFORMATION}.
+   *
+   * @param parent the key in {@link NodeEntry.LifecycleState#CHECK_DEPENDENCIES}
+   * @param deps the {@linkplain NodeEntry#getNextDirtyDirectDeps next dirty dep group} of {@code
+   *     parent}; only called when all previous dep groups were clean, so it is known that {@code
+   *     deps} are still dependencies of {@code parent} on the incremental build
    */
-  DepsReport analyzeDepsDoneness(SkyKey parent, Collection<SkyKey> deps)
-      throws InterruptedException;
+  DepsReport analyzeDepsDoneness(SkyKey parent, List<SkyKey> deps) throws InterruptedException;
 }
diff --git a/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java b/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java
index 2c7da6d7f030c5..fe94c23c0759f2 100644
--- a/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java
+++ b/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java
@@ -21,7 +21,6 @@
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
 import com.google.devtools.build.skyframe.NodeEntry.DirtyType;
 import com.google.errorprone.annotations.ForOverride;
-import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -132,7 +131,7 @@ public ImmutableSet<SkyKey> prefetchDeps(
     }
 
     @Override
-    public DepsReport analyzeDepsDoneness(SkyKey parent, Collection<SkyKey> deps)
+    public DepsReport analyzeDepsDoneness(SkyKey parent, List<SkyKey> deps)
         throws InterruptedException {
       return delegate.analyzeDepsDoneness(parent, deps);
     }