Refactor DirtyBuildingState to allow custom implementation on storing the

dependencies used in last build

--
PiperOrigin-RevId: 146169454
MOS_MIGRATED_REVID=146169454

4 files changed, 61 insertions, 40 deletions

diff --git a/src/main/java/com/google/devtools/build/skyframe/DelegatingNodeEntry.java b/src/main/java/com/google/devtools/build/skyframe/DelegatingNodeEntry.java
index 2152fb34b0..f0003f7be2 100644
--- a/src/main/java/com/google/devtools/build/skyframe/DelegatingNodeEntry.java
+++ b/src/main/java/com/google/devtools/build/skyframe/DelegatingNodeEntry.java
@@ -106,17 +106,17 @@ public abstract class DelegatingNodeEntry implements NodeEntry {
   }
 
   @Override
-  public Collection<SkyKey> getNextDirtyDirectDeps() {
+  public Collection<SkyKey> getNextDirtyDirectDeps() throws InterruptedException {
     return getDelegate().getNextDirtyDirectDeps();
   }
 
   @Override
-  public Iterable<SkyKey> getAllDirectDepsForIncompleteNode() {
+  public Iterable<SkyKey> getAllDirectDepsForIncompleteNode() throws InterruptedException {
     return getDelegate().getAllDirectDepsForIncompleteNode();
   }
 
   @Override
-  public Set<SkyKey> getAllRemainingDirtyDirectDeps() {
+  public Set<SkyKey> getAllRemainingDirtyDirectDeps() throws InterruptedException {
     return getDelegate().getAllRemainingDirtyDirectDeps();
   }
 
diff --git a/src/main/java/com/google/devtools/build/skyframe/DirtyBuildingState.java b/src/main/java/com/google/devtools/build/skyframe/DirtyBuildingState.java
index a476275d00..2c34feb107 100644
--- a/src/main/java/com/google/devtools/build/skyframe/DirtyBuildingState.java
+++ b/src/main/java/com/google/devtools/build/skyframe/DirtyBuildingState.java
@@ -44,7 +44,14 @@ public abstract class DirtyBuildingState extends BuildingState {
    * it means that this node is being built for the first time. See {@link
    * InMemoryNodeEntry#directDeps} for more on dependency group storage.
    */
-  protected final GroupedList<SkyKey> lastBuildDirectDeps;
+  protected abstract GroupedList<SkyKey> getLastBuildDirectDeps() throws InterruptedException;
+
+  /**
+   * The number of groups of the dependencies requested last time when the node was built.
+   *
+   * <p>Getting the number of last-built dependencies should not throw {@link InterruptedException}.
+   */
+  protected abstract int getNumOfGroupsInLastBuildDirectDeps();
 
   /** The value of the node the last time it was built. */
   protected abstract SkyValue getLastBuildValue() throws InterruptedException;
@@ -55,12 +62,7 @@ public abstract class DirtyBuildingState extends BuildingState {
    */
   private int dirtyDirectDepIndex = -1;
 
-  protected DirtyBuildingState(boolean isChanged, GroupedList<SkyKey> lastBuildDirectDeps) {
-    this.lastBuildDirectDeps = lastBuildDirectDeps;
-    Preconditions.checkState(
-        isChanged || !this.lastBuildDirectDeps.isEmpty(),
-        "%s is being marked dirty, not changed, but has no children that could have dirtied it",
-        this);
+  protected DirtyBuildingState(boolean isChanged) {
     dirtyState = isChanged ? DirtyState.NEEDS_REBUILDING : DirtyState.CHECK_DEPENDENCIES;
     // We need to iterate through the deps to see if they have changed, or to remove them if one
     // has. Initialize the iterating index.
@@ -69,7 +71,7 @@ public abstract class DirtyBuildingState extends BuildingState {
 
   static BuildingState create(
       boolean isChanged, GroupedList<SkyKey> lastBuildDirectDeps, SkyValue lastBuildValue) {
-    return new DirtyBuildingStateWithValue(isChanged, lastBuildDirectDeps, lastBuildValue);
+    return new FullDirtyBuildingState(isChanged, lastBuildDirectDeps, lastBuildValue);
   }
 
   final void markChanged() {
@@ -83,7 +85,7 @@ public abstract class DirtyBuildingState extends BuildingState {
     Preconditions.checkState(isDirty(), this);
     Preconditions.checkState(!isChanged(), this);
     Preconditions.checkState(isEvaluating(), this);
-    Preconditions.checkState(lastBuildDirectDeps.listSize() == dirtyDirectDepIndex, this);
+    Preconditions.checkState(getNumOfGroupsInLastBuildDirectDeps() == dirtyDirectDepIndex, this);
     dirtyState = DirtyState.REBUILDING;
   }
 
@@ -117,7 +119,7 @@ public abstract class DirtyBuildingState extends BuildingState {
    * DirtyState#NEEDS_REBUILDING} if the child has changed, and {@link DirtyState#VERIFIED_CLEAN} if
    * the child has not changed and this was the last child to be checked (as determined by {@link
    * #isReady} and comparing {@link #dirtyDirectDepIndex} and {@link
-   * DirtyBuildingState#lastBuildDirectDeps#listSize}.
+   * DirtyBuildingState#getNumOfGroupsInLastBuildDirectDeps()}.
    */
   @Override
   final void signalDepInternal(boolean childChanged, int numDirectDeps) {
@@ -128,7 +130,7 @@ public abstract class DirtyBuildingState extends BuildingState {
         dirtyState = DirtyState.NEEDS_REBUILDING;
       } else if (dirtyState == DirtyState.CHECK_DEPENDENCIES
           && isReady(numDirectDeps)
-          && lastBuildDirectDeps.listSize() == dirtyDirectDepIndex) {
+          && getNumOfGroupsInLastBuildDirectDeps() == dirtyDirectDepIndex) {
         // No other dep already marked this as NEEDS_REBUILDING, no deps outstanding, and this was
         // the last block of deps to be checked.
         dirtyState = DirtyState.VERIFIED_CLEAN;
@@ -152,13 +154,14 @@ public abstract class DirtyBuildingState extends BuildingState {
    * Returns true if the deps requested during this evaluation ({@code directDeps}) are exactly
    * those requested the last time this node was built, in the same order.
    */
-  final boolean depsUnchangedFromLastBuild(GroupedList<SkyKey> directDeps) {
+  final boolean depsUnchangedFromLastBuild(GroupedList<SkyKey> directDeps)
+      throws InterruptedException {
     checkFinishedBuildingWhenAboutToSetValue();
-    return lastBuildDirectDeps.equals(directDeps);
+    return getLastBuildDirectDeps().equals(directDeps);
   }
 
   final boolean noDepsLastBuild() {
-    return lastBuildDirectDeps.isEmpty();
+    return getNumOfGroupsInLastBuildDirectDeps() == 0;
   }
 
   /**
@@ -179,12 +182,12 @@ public abstract class DirtyBuildingState extends BuildingState {
    *
    * <p>See {@link NodeEntry#getNextDirtyDirectDeps}.
    */
-  final Collection<SkyKey> getNextDirtyDirectDeps() {
+  final Collection<SkyKey> getNextDirtyDirectDeps() throws InterruptedException {
     Preconditions.checkState(isDirty(), this);
     Preconditions.checkState(dirtyState == DirtyState.CHECK_DEPENDENCIES, this);
     Preconditions.checkState(isEvaluating(), this);
-    Preconditions.checkState(dirtyDirectDepIndex < lastBuildDirectDeps.listSize(), this);
-    return lastBuildDirectDeps.get(dirtyDirectDepIndex++);
+    Preconditions.checkState(dirtyDirectDepIndex < getNumOfGroupsInLastBuildDirectDeps(), this);
+    return getLastBuildDirectDeps().get(dirtyDirectDepIndex++);
   }
 
   /**
@@ -192,15 +195,15 @@ public abstract class DirtyBuildingState extends BuildingState {
    * true, this method is non-mutating. If {@code preservePosition} is false, the caller must
    * process the returned set, and so subsequent calls to this method will return the empty set.
    */
-  Set<SkyKey> getAllRemainingDirtyDirectDeps(boolean preservePosition) {
+  Set<SkyKey> getAllRemainingDirtyDirectDeps(boolean preservePosition) throws InterruptedException {
     Preconditions.checkState(isDirty(), this);
     ImmutableSet.Builder<SkyKey> result = ImmutableSet.builder();
 
-    for (int ind = dirtyDirectDepIndex; ind < lastBuildDirectDeps.listSize(); ind++) {
-      result.addAll(lastBuildDirectDeps.get(ind));
+    for (int ind = dirtyDirectDepIndex; ind < getNumOfGroupsInLastBuildDirectDeps(); ind++) {
+      result.addAll(getLastBuildDirectDeps().get(ind));
     }
     if (!preservePosition) {
-      dirtyDirectDepIndex = lastBuildDirectDeps.listSize();
+      dirtyDirectDepIndex = getNumOfGroupsInLastBuildDirectDeps();
     }
     return result.build();
   }
@@ -214,16 +217,21 @@ public abstract class DirtyBuildingState extends BuildingState {
   protected MoreObjects.ToStringHelper getStringHelper() {
     return super.getStringHelper()
         .add("dirtyState", dirtyState)
-        .add("lastBuildDirectDeps", lastBuildDirectDeps)
         .add("dirtyDirectDepIndex", dirtyDirectDepIndex);
   }
 
-  private static class DirtyBuildingStateWithValue extends DirtyBuildingState {
+  private static class FullDirtyBuildingState extends DirtyBuildingState {
+    private final GroupedList<SkyKey> lastBuildDirectDeps;
     private final SkyValue lastBuildValue;
 
-    private DirtyBuildingStateWithValue(
+    private FullDirtyBuildingState(
         boolean isChanged, GroupedList<SkyKey> lastBuildDirectDeps, SkyValue lastBuildValue) {
-      super(isChanged, lastBuildDirectDeps);
+      super(isChanged);
+      this.lastBuildDirectDeps = lastBuildDirectDeps;
+      Preconditions.checkState(
+          isChanged || getNumOfGroupsInLastBuildDirectDeps() > 0,
+          "%s is being marked dirty, not changed, but has no children that could have dirtied it",
+          this);
       this.lastBuildValue = lastBuildValue;
     }
 
@@ -233,8 +241,20 @@ public abstract class DirtyBuildingState extends BuildingState {
     }
 
     @Override
+    protected GroupedList<SkyKey> getLastBuildDirectDeps() throws InterruptedException {
+      return lastBuildDirectDeps;
+    }
+
+    @Override
+    protected int getNumOfGroupsInLastBuildDirectDeps() {
+      return lastBuildDirectDeps.listSize();
+    }
+
+    @Override
     protected MoreObjects.ToStringHelper getStringHelper() {
-      return super.getStringHelper().add("lastBuildValue", lastBuildValue);
+      return super.getStringHelper()
+          .add("lastBuildDirectDeps", lastBuildDirectDeps)
+          .add("lastBuildValue", lastBuildValue);
     }
   }
 }
diff --git a/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java b/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java
index 5d8dbb3f4b..fefee27763 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java
@@ -405,13 +405,14 @@ public class InMemoryNodeEntry implements NodeEntry {
 
   /** @see DirtyBuildingState#getNextDirtyDirectDeps() */
   @Override
-  public synchronized Collection<SkyKey> getNextDirtyDirectDeps() {
+  public synchronized Collection<SkyKey> getNextDirtyDirectDeps() throws InterruptedException {
     Preconditions.checkState(isReady(), this);
     return getDirtyBuildingState().getNextDirtyDirectDeps();
   }
 
   @Override
-  public synchronized Iterable<SkyKey> getAllDirectDepsForIncompleteNode() {
+  public synchronized Iterable<SkyKey> getAllDirectDepsForIncompleteNode()
+      throws InterruptedException {
     Preconditions.checkState(!isDone(), this);
     if (!isDirty()) {
       return Iterables.concat(getTemporaryDirectDeps());
@@ -428,7 +429,7 @@ public class InMemoryNodeEntry implements NodeEntry {
   }
 
   @Override
-  public synchronized Set<SkyKey> getAllRemainingDirtyDirectDeps() {
+  public synchronized Set<SkyKey> getAllRemainingDirtyDirectDeps() throws InterruptedException {
     if (isDirty()) {
       Preconditions.checkState(
           getDirtyBuildingState().getDirtyState() == DirtyState.REBUILDING, this);
diff --git a/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java b/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java
index b86d4712a4..d7f7fcfc89 100644
--- a/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java
+++ b/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java
@@ -284,16 +284,16 @@ public interface NodeEntry extends ThinNodeEntry {
    * @see DirtyBuildingState#getNextDirtyDirectDeps()
    */
   @ThreadSafe
-  Collection<SkyKey> getNextDirtyDirectDeps();
+  Collection<SkyKey> getNextDirtyDirectDeps() throws InterruptedException;
 
   /**
    * Returns all deps of a node that has not yet finished evaluating. In other words, if a node has
    * a reverse dep on this node, its key will be in the returned set here. If this node was freshly
    * created, this is just any elements that were added using {@link #addTemporaryDirectDeps} (so it
    * is the same as {@link #getTemporaryDirectDeps}). If this node is marked dirty, this includes
-   * all the elements that would have been returned by successive calls to
-   * {@link #getNextDirtyDirectDeps} (or, equivalently, one call to
-   * {@link #getAllRemainingDirtyDirectDeps}).
+   * all the elements that would have been returned by successive calls to {@link
+   * #getNextDirtyDirectDeps} (or, equivalently, one call to {@link
+   * #getAllRemainingDirtyDirectDeps}).
    *
    * <p>This method should only be called when this node is about to be deleted after an aborted
    * evaluation. After such an evaluation, any nodes that did not finish evaluating are deleted, as
@@ -305,7 +305,7 @@ public interface NodeEntry extends ThinNodeEntry {
    * <p>This method must not be called twice: the next thing done to this node after this method is
    * called should be the removal of the node from the graph.
    */
-  Iterable<SkyKey> getAllDirectDepsForIncompleteNode();
+  Iterable<SkyKey> getAllDirectDepsForIncompleteNode() throws InterruptedException;
 
   /**
    * If an entry {@link #isDirty}, returns all direct deps that were present last build, but have
@@ -319,10 +319,10 @@ public interface NodeEntry extends ThinNodeEntry {
    * started evaluation.
    *
    * <p>This method does not mutate the entry. In particular, multiple calls to this method will
-   * always produce the same result until the entry finishes evaluation. Contrast with
-   * {@link #getAllDirectDepsForIncompleteNode}.
+   * always produce the same result until the entry finishes evaluation. Contrast with {@link
+   * #getAllDirectDepsForIncompleteNode}.
    */
-  Set<SkyKey> getAllRemainingDirtyDirectDeps();
+  Set<SkyKey> getAllRemainingDirtyDirectDeps() throws InterruptedException;
 
   /**
    * Notifies a node that it is about to be rebuilt. This method can only be called if the node