Stop converting temporary direct deps to a set. In almost all cases, this conversion is unnecessary and wasteful. In the remaining cases, the set conversion can be explicit.

--
MOS_MIGRATED_REVID=122294939

4 files changed, 27 insertions, 22 deletions

diff --git a/src/test/java/com/google/devtools/build/skyframe/DeterministicGraph.java b/src/test/java/com/google/devtools/build/skyframe/DeterministicGraph.java
index 81149777c6..85df5c59d5 100644
--- a/src/test/java/com/google/devtools/build/skyframe/DeterministicGraph.java
+++ b/src/test/java/com/google/devtools/build/skyframe/DeterministicGraph.java
@@ -121,12 +121,5 @@ public class DeterministicGraph<TGraph extends ThinNodeQueryableGraph>
       result.addAll(super.getInProgressReverseDeps());
       return result;
     }
-
-    @Override
-    public synchronized Set<SkyKey> getTemporaryDirectDeps() {
-      TreeSet<SkyKey> result = new TreeSet<>(ALPHABETICAL_SKYKEY_COMPARATOR);
-      result.addAll(super.getTemporaryDirectDeps());
-      return result;
-    }
   }
 }
diff --git a/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java b/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java
index 4170c54922..92bed7b9b4 100644
--- a/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java
@@ -77,7 +77,7 @@ public class InMemoryNodeEntryTest {
     assertFalse(entry.isReady());
     assertTrue(entry.signalDep());
     assertTrue(entry.isReady());
-    assertThat(entry.getTemporaryDirectDeps()).containsExactly(dep1);
+    assertThatNodeEntry(entry).hasTemporaryDirectDepsThat().containsExactly(dep1);
     SkyKey dep2 = key("dep2");
     SkyKey dep3 = key("dep3");
     addTemporaryDirectDep(entry, dep2);
@@ -197,7 +197,7 @@ public class InMemoryNodeEntryTest {
     assertThat(entry.getNextDirtyDirectDeps()).containsExactly(dep);
     addTemporaryDirectDep(entry, dep);
     entry.signalDep();
-    assertThat(entry.getTemporaryDirectDeps()).containsExactly(dep);
+    assertThatNodeEntry(entry).hasTemporaryDirectDepsThat().containsExactly(dep);
     assertTrue(entry.isReady());
     assertThat(entry.markRebuildingAndGetAllRemainingDirtyDirectDeps()).isEmpty();
     assertThat(setValue(entry, new SkyValue() {}, /*errorInfo=*/null,
@@ -414,7 +414,7 @@ public class InMemoryNodeEntryTest {
     addTemporaryDirectDep(entry, dep);
     entry.signalDep(IntVersion.of(1L));
     assertEquals(NodeEntry.DirtyState.NEEDS_REBUILDING, entry.getDirtyState());
-    assertThat(entry.getTemporaryDirectDeps()).containsExactly(dep);
+    assertThatNodeEntry(entry).hasTemporaryDirectDepsThat().containsExactly(dep);
     assertThat(entry.markRebuildingAndGetAllRemainingDirtyDirectDeps()).isEmpty();
     setValue(entry, new IntegerValue(5), /*errorInfo=*/null, /*graphVersion=*/1L);
     assertTrue(entry.isDone());
@@ -443,7 +443,7 @@ public class InMemoryNodeEntryTest {
     addTemporaryDirectDep(entry, dep);
     entry.signalDep(IntVersion.of(1L));
     assertEquals(NodeEntry.DirtyState.NEEDS_REBUILDING, entry.getDirtyState());
-    assertThat(entry.getTemporaryDirectDeps()).containsExactly(dep);
+    assertThatNodeEntry(entry).hasTemporaryDirectDepsThat().containsExactly(dep);
     ReifiedSkyFunctionException exception = new ReifiedSkyFunctionException(
         new GenericFunctionException(new SomeErrorException("oops"), Transience.PERSISTENT),
         key("cause"));
@@ -480,7 +480,7 @@ public class InMemoryNodeEntryTest {
     addTemporaryDirectDep(entry, dep);
     entry.signalDep(IntVersion.of(1L));
     assertEquals(NodeEntry.DirtyState.NEEDS_REBUILDING, entry.getDirtyState());
-    assertThat(entry.getTemporaryDirectDeps()).containsExactly(dep);
+    assertThatNodeEntry(entry).hasTemporaryDirectDepsThat().containsExactly(dep);
     assertThat(entry.markRebuildingAndGetAllRemainingDirtyDirectDeps())
         .containsExactly(dep1InGroup, dep2InGroup);
     addTemporaryDirectDeps(entry, dep2InGroup, dep1InGroup);
@@ -511,7 +511,7 @@ public class InMemoryNodeEntryTest {
     addTemporaryDirectDep(entry, dep);
     entry.signalDep(IntVersion.of(1L));
     assertEquals(NodeEntry.DirtyState.NEEDS_REBUILDING, entry.getDirtyState());
-    assertThat(entry.getTemporaryDirectDeps()).containsExactly(dep);
+    assertThatNodeEntry(entry).hasTemporaryDirectDepsThat().containsExactly(dep);
     assertThat(entry.markRebuildingAndGetAllRemainingDirtyDirectDeps()).isEmpty();
     setValue(entry, /*value=*/null, errorInfo, /*graphVersion=*/1L);
     assertTrue(entry.isDone());
@@ -588,7 +588,7 @@ public class InMemoryNodeEntryTest {
     addTemporaryDirectDep(entry, dep);
     assertTrue(entry.signalDep(IntVersion.of(1L)));
     assertEquals(NodeEntry.DirtyState.NEEDS_REBUILDING, entry.getDirtyState());
-    assertThat(entry.getTemporaryDirectDeps()).containsExactly(dep);
+    assertThatNodeEntry(entry).hasTemporaryDirectDepsThat().containsExactly(dep);
     assertThat(entry.markRebuildingAndGetAllRemainingDirtyDirectDeps()).isEmpty();
     addTemporaryDirectDep(entry, key("dep2"));
     assertTrue(entry.signalDep(IntVersion.of(1L)));
diff --git a/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java b/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
index 8ca696e9e7..c18f07666d 100644
--- a/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
@@ -1047,9 +1047,8 @@ public class MemoizingEvaluatorTest {
 
   @Test
   public void cycleAndSelfEdgeWithDirtyValueInSameGroup() throws Exception {
-    makeGraphDeterministic();
-    final SkyKey cycleKey1 = GraphTester.toSkyKey("zcycleKey1");
-    final SkyKey cycleKey2 = GraphTester.toSkyKey("acycleKey2");
+    final SkyKey cycleKey1 = GraphTester.toSkyKey("cycleKey1");
+    final SkyKey cycleKey2 = GraphTester.toSkyKey("cycleKey2");
     tester.getOrCreate(cycleKey2).addDependency(cycleKey2).setComputedValue(CONCATENATE);
     tester
         .getOrCreate(cycleKey1)
@@ -1059,8 +1058,9 @@ public class MemoizingEvaluatorTest {
               @Override
               public SkyValue compute(SkyKey skyKey, Environment env)
                   throws SkyFunctionException, InterruptedException {
+                // The order here is important -- 2 before 1.
                 Map<SkyKey, SkyValue> result =
-                    env.getValues(ImmutableList.of(cycleKey1, cycleKey2));
+                    env.getValues(ImmutableList.of(cycleKey2, cycleKey1));
                 Preconditions.checkState(env.valuesMissing(), result);
                 return null;
               }
diff --git a/src/test/java/com/google/devtools/build/skyframe/NodeEntrySubject.java b/src/test/java/com/google/devtools/build/skyframe/NodeEntrySubject.java
index 537d738c94..50ed287a19 100644
--- a/src/test/java/com/google/devtools/build/skyframe/NodeEntrySubject.java
+++ b/src/test/java/com/google/devtools/build/skyframe/NodeEntrySubject.java
@@ -13,21 +13,33 @@
 // limitations under the License.
 package com.google.devtools.build.skyframe;
 
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.Iterables;
 import com.google.common.truth.DefaultSubject;
 import com.google.common.truth.FailureStrategy;
+import com.google.common.truth.IterableSubject;
 import com.google.common.truth.Subject;
-import com.google.common.truth.Truth;
 
 /**
  * {@link Subject} for {@link NodeEntry}. Please add to this class if you need more
  * functionality!
  */
-public class NodeEntrySubject extends Subject<NodeEntrySubject, NodeEntry> {
-  public NodeEntrySubject(FailureStrategy failureStrategy, NodeEntry nodeEntry) {
+class NodeEntrySubject extends Subject<NodeEntrySubject, NodeEntry> {
+  NodeEntrySubject(FailureStrategy failureStrategy, NodeEntry nodeEntry) {
     super(failureStrategy, nodeEntry);
   }
 
   DefaultSubject hasVersionThat() {
-    return Truth.assertThat(getSubject().getVersion()).named("Version for " + getDisplaySubject());
+    return assertThat(getSubject().getVersion()).named(detail("Version"));
+  }
+
+  IterableSubject hasTemporaryDirectDepsThat() {
+    return assertThat(Iterables.concat(getSubject().getTemporaryDirectDeps()))
+        .named(detail("TemporaryDirectDeps"));
+  }
+
+  private String detail(String descriptor) {
+    return descriptor + " for" + getDisplaySubject();
   }
 }