Augment the QueryableGraph#get[BatchWithFieldHints] method to take in parameters conveying the requesting node (if any), the requested node(s), as well as a reason for the skyframe graph lookup. Alternate graph implementations may be interested in this information.

--
MOS_MIGRATED_REVID=128496089

6 files changed, 75 insertions, 57 deletions

diff --git a/src/test/java/com/google/devtools/build/skyframe/DeterministicHelper.java b/src/test/java/com/google/devtools/build/skyframe/DeterministicHelper.java
index 50bd380ed7..fe7c76fc91 100644
--- a/src/test/java/com/google/devtools/build/skyframe/DeterministicHelper.java
+++ b/src/test/java/com/google/devtools/build/skyframe/DeterministicHelper.java
@@ -14,7 +14,6 @@
 package com.google.devtools.build.skyframe;
 
 import com.google.common.collect.Iterables;
-
 import java.util.Collection;
 import java.util.Comparator;
 import java.util.EnumSet;
@@ -91,8 +90,8 @@ public class DeterministicHelper extends NotifyingHelper {
     }
 
     @Override
-    public Map<SkyKey, NodeEntry> getBatch(Iterable<SkyKey> keys) {
-      return makeDeterministic(super.getBatch(keys));
+    public Map<SkyKey, NodeEntry> getBatchForInvalidation(Iterable<SkyKey> keys) {
+      return makeDeterministic(super.getBatchForInvalidation(keys));
     }
   }
 
@@ -111,14 +110,18 @@ public class DeterministicHelper extends NotifyingHelper {
     }
 
     @Override
-    public Map<SkyKey, NodeEntry> createIfAbsentBatch(Iterable<SkyKey> keys) {
-      return makeDeterministic(super.createIfAbsentBatch(keys));
+    public Map<SkyKey, NodeEntry> createIfAbsentBatch(
+        @Nullable SkyKey requestor, Reason reason, Iterable<SkyKey> keys) {
+      return makeDeterministic(super.createIfAbsentBatch(requestor, reason, keys));
     }
 
     @Override
     public Map<SkyKey, NodeEntry> getBatchWithFieldHints(
-        Iterable<SkyKey> keys, EnumSet<NodeEntryField> fields) {
-      return makeDeterministic(super.getBatchWithFieldHints(keys, fields));
+        @Nullable SkyKey requestor,
+        Reason reason,
+        Iterable<SkyKey> keys,
+        EnumSet<NodeEntryField> fields) {
+      return makeDeterministic(super.getBatchWithFieldHints(requestor, reason, keys, fields));
     }
   }
 
diff --git a/src/test/java/com/google/devtools/build/skyframe/DeterministicInMemoryGraph.java b/src/test/java/com/google/devtools/build/skyframe/DeterministicInMemoryGraph.java
index bb61d0f202..0cbc91ce32 100644
--- a/src/test/java/com/google/devtools/build/skyframe/DeterministicInMemoryGraph.java
+++ b/src/test/java/com/google/devtools/build/skyframe/DeterministicInMemoryGraph.java
@@ -27,8 +27,8 @@ class DeterministicInMemoryGraph extends DeterministicHelper.DeterministicProces
   }
 
   @Override
-  public Map<SkyKey, NodeEntry> getBatch(Iterable<SkyKey> keys) {
-    return getBatchWithFieldHints(keys, NodeEntryField.ALL_FIELDS);
+  public Map<SkyKey, NodeEntry> getBatchForInvalidation(Iterable<SkyKey> keys) {
+    return getBatchWithFieldHints(null, Reason.INVALIDATION, keys, NodeEntryField.ALL_FIELDS);
   }
 
   @Override
diff --git a/src/test/java/com/google/devtools/build/skyframe/EagerInvalidatorTest.java b/src/test/java/com/google/devtools/build/skyframe/EagerInvalidatorTest.java
index dbc418f79f..fb3058c103 100644
--- a/src/test/java/com/google/devtools/build/skyframe/EagerInvalidatorTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/EagerInvalidatorTest.java
@@ -39,7 +39,7 @@ import com.google.devtools.build.skyframe.InvalidatingNodeVisitor.DirtyingInvali
 import com.google.devtools.build.skyframe.InvalidatingNodeVisitor.DirtyingNodeVisitor;
 import com.google.devtools.build.skyframe.InvalidatingNodeVisitor.InvalidationState;
 import com.google.devtools.build.skyframe.InvalidatingNodeVisitor.InvalidationType;
-
+import com.google.devtools.build.skyframe.QueryableGraph.Reason;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -93,7 +93,7 @@ public class EagerInvalidatorTest {
   boolean gcExpected() { throw new UnsupportedOperationException(); }
 
   private boolean isInvalidated(SkyKey key) {
-    NodeEntry entry = graph.get(key);
+    NodeEntry entry = graph.get(null, Reason.OTHER, key);
     if (gcExpected()) {
       return entry == null;
     } else {
@@ -102,7 +102,7 @@ public class EagerInvalidatorTest {
   }
 
   private void assertChanged(SkyKey key) {
-    NodeEntry entry = graph.get(key);
+    NodeEntry entry = graph.get(null, Reason.OTHER, key);
     if (gcExpected()) {
       assertNull(entry);
     } else {
@@ -111,7 +111,7 @@ public class EagerInvalidatorTest {
   }
 
   private void assertDirtyAndNotChanged(SkyKey key) {
-    NodeEntry entry = graph.get(key);
+    NodeEntry entry = graph.get(null, Reason.OTHER, key);
     if (gcExpected()) {
       assertNull(entry);
     } else {
@@ -344,10 +344,12 @@ public class EagerInvalidatorTest {
         .setComputedValue(CONCATENATE);
     eval(false, skyKey("ab_c"), skyKey("bc"));
 
-    assertThat(graph.get(skyKey("a")).getReverseDeps()).containsExactly(skyKey("ab"));
-    assertThat(graph.get(skyKey("b")).getReverseDeps()).containsExactly(skyKey("ab"), skyKey("bc"));
-    assertThat(graph.get(skyKey("c")).getReverseDeps()).containsExactly(skyKey("ab_c"),
-        skyKey("bc"));
+    assertThat(graph.get(null, Reason.OTHER, skyKey("a"))
+        .getReverseDeps()).containsExactly(skyKey("ab"));
+    assertThat(graph.get(null, Reason.OTHER, skyKey("b"))
+        .getReverseDeps()).containsExactly(skyKey("ab"), skyKey("bc"));
+    assertThat(graph.get(null, Reason.OTHER, skyKey("c"))
+        .getReverseDeps()).containsExactly(skyKey("ab_c"), skyKey("bc"));
 
     invalidateWithoutError(null, skyKey("ab"));
     eval(false);
@@ -361,15 +363,18 @@ public class EagerInvalidatorTest {
     if (reverseDepsPresent()) {
       reverseDeps.add(skyKey("ab"));
     }
-    assertThat(graph.get(skyKey("a")).getReverseDeps()).containsExactlyElementsIn(reverseDeps);
+    assertThat(graph.get(null, Reason.OTHER, skyKey("a"))
+        .getReverseDeps()).containsExactlyElementsIn(reverseDeps);
     reverseDeps.add(skyKey("bc"));
-    assertThat(graph.get(skyKey("b")).getReverseDeps()).containsExactlyElementsIn(reverseDeps);
+    assertThat(graph.get(null, Reason.OTHER, skyKey("b"))
+        .getReverseDeps()).containsExactlyElementsIn(reverseDeps);
     reverseDeps.clear();
     if (reverseDepsPresent()) {
       reverseDeps.add(skyKey("ab_c"));
     }
     reverseDeps.add(skyKey("bc"));
-    assertThat(graph.get(skyKey("c")).getReverseDeps()).containsExactlyElementsIn(reverseDeps);
+    assertThat(graph.get(null, Reason.OTHER, skyKey("c"))
+        .getReverseDeps()).containsExactlyElementsIn(reverseDeps);
   }
 
   @Test
@@ -438,7 +443,7 @@ public class EagerInvalidatorTest {
     assertFalse(state.isEmpty());
     final Set<SkyKey> invalidated = Sets.newConcurrentHashSet();
     assertFalse(isInvalidated(parent));
-    assertNotNull(graph.get(parent).getValue());
+    assertNotNull(graph.get(null, Reason.OTHER, parent).getValue());
     receiver = new EvaluationProgressReceiver() {
       @Override
       public void invalidated(SkyKey skyKey, InvalidationState state) {
diff --git a/src/test/java/com/google/devtools/build/skyframe/GraphConcurrencyTest.java b/src/test/java/com/google/devtools/build/skyframe/GraphConcurrencyTest.java
index 73e42ea4d7..df6f99ca22 100644
--- a/src/test/java/com/google/devtools/build/skyframe/GraphConcurrencyTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/GraphConcurrencyTest.java
@@ -30,7 +30,7 @@ import com.google.devtools.build.lib.util.GroupedList.GroupedListHelper;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.skyframe.GraphTester.StringValue;
 import com.google.devtools.build.skyframe.NodeEntry.DependencyState;
-
+import com.google.devtools.build.skyframe.QueryableGraph.Reason;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -77,7 +77,7 @@ public abstract class GraphConcurrencyTest {
 
   @Test
   public void createIfAbsentBatchSanity() {
-    graph.createIfAbsentBatch(ImmutableList.of(key("cat"), key("dog")));
+    graph.createIfAbsentBatch(null, Reason.OTHER, ImmutableList.of(key("cat"), key("dog")));
   }
 
   @Test
@@ -91,11 +91,11 @@ public abstract class GraphConcurrencyTest {
                   new Runnable() {
                     @Override
                     public void run() {
-                      graph.get(key);
+                      graph.get(null, Reason.OTHER, key);
                     }
                   }));
       t.start();
-      assertThat(graph.createIfAbsentBatch(ImmutableList.of(key))).isNotEmpty();
+      assertThat(graph.createIfAbsentBatch(null, Reason.OTHER, ImmutableList.of(key))).isNotEmpty();
       graph.remove(key);
     }
   }
@@ -111,7 +111,7 @@ public abstract class GraphConcurrencyTest {
           public void run() {
             TrackingAwaiter.INSTANCE.awaitLatchAndTrackExceptions(
                 startThreads, "threads not started");
-            graph.createIfAbsentBatch(ImmutableList.of(key));
+            graph.createIfAbsentBatch(null, Reason.OTHER, ImmutableList.of(key));
           }
         };
     Runnable noCreateRunnable =
@@ -120,7 +120,7 @@ public abstract class GraphConcurrencyTest {
           public void run() {
             TrackingAwaiter.INSTANCE.awaitLatchAndTrackExceptions(
                 startThreads, "threads not started");
-            graph.get(key);
+            graph.get(null, Reason.OTHER, key);
           }
         };
     List<Thread> threads = new ArrayList<>(2 * numThreads);
@@ -144,7 +144,7 @@ public abstract class GraphConcurrencyTest {
   public void testAddRemoveRdeps() throws Exception {
     SkyKey key = key("foo");
     final NodeEntry entry = Iterables.getOnlyElement(
-        graph.createIfAbsentBatch(ImmutableList.of(key)).values());
+        graph.createIfAbsentBatch(null, Reason.OTHER, ImmutableList.of(key)).values());
     // These numbers are arbitrary.
     int numThreads = 50;
     int numKeys = numThreads;
@@ -169,7 +169,7 @@ public abstract class GraphConcurrencyTest {
     for (int i = 0; i < numKeys; i++) {
       rdepKeys.add(key("rdep" + i));
     }
-    graph.createIfAbsentBatch(rdepKeys);
+    graph.createIfAbsentBatch(null, Reason.OTHER, rdepKeys);
     for (int i = 0; i < numKeys; i++) {
       final int j = i;
       Runnable r =
@@ -200,18 +200,18 @@ public abstract class GraphConcurrencyTest {
     waitForSetValue.countDown();
     wrapper.waitForTasksAndMaybeThrow();
     assertFalse(ExecutorUtil.interruptibleShutdown(pool));
-    assertEquals(new StringValue("foo1"), graph.get(key).getValue());
-    assertEquals(numKeys + 1, Iterables.size(graph.get(key).getReverseDeps()));
+    assertEquals(new StringValue("foo1"), graph.get(null, Reason.OTHER, key).getValue());
+    assertEquals(numKeys + 1, Iterables.size(graph.get(null, Reason.OTHER, key).getReverseDeps()));
 
     graph = getGraph(getNextVersion(startingVersion));
-    NodeEntry sameEntry = Preconditions.checkNotNull(graph.get(key));
+    NodeEntry sameEntry = Preconditions.checkNotNull(graph.get(null, Reason.OTHER, key));
     // Mark the node as dirty again and check that the reverse deps have been preserved.
     sameEntry.markDirty(true);
     startEvaluation(sameEntry);
     sameEntry.markRebuilding();
     sameEntry.setValue(new StringValue("foo2"), getNextVersion(startingVersion));
-    assertEquals(new StringValue("foo2"), graph.get(key).getValue());
-    assertEquals(numKeys + 1, Iterables.size(graph.get(key).getReverseDeps()));
+    assertEquals(new StringValue("foo2"), graph.get(null, Reason.OTHER, key).getValue());
+    assertEquals(numKeys + 1, Iterables.size(graph.get(null, Reason.OTHER, key).getReverseDeps()));
   }
 
   // Tests adding inflight nodes with a given key while an existing node with the same key
@@ -236,12 +236,13 @@ public abstract class GraphConcurrencyTest {
               new Runnable() {
                 public void run() {
                   for (SkyKey key : keys) {
-                    NodeEntry entry = graph.get(key);
+                    NodeEntry entry = graph.get(null, Reason.OTHER, key);
                     if (entry == null) {
                       nodeCreated.add(key);
                     }
                   }
-                  Map<SkyKey, NodeEntry> entries = graph.createIfAbsentBatch(keys);
+                  Map<SkyKey, NodeEntry> entries =
+                      graph.createIfAbsentBatch(null, Reason.OTHER, keys);
                   for (Integer keyNum : ImmutableList.of(keyNum1, keyNum2)) {
                     SkyKey key = key("foo" + keyNum);
                     NodeEntry entry = entries.get(key);
@@ -255,7 +256,7 @@ public abstract class GraphConcurrencyTest {
                     }
                   }
                   // This shouldn't cause any problems from the other threads.
-                  graph.createIfAbsentBatch(keys);
+                  graph.createIfAbsentBatch(null, Reason.OTHER, keys);
                 }
               };
           pool.execute(wrapper.wrap(r));
@@ -269,8 +270,9 @@ public abstract class GraphConcurrencyTest {
       SkyKey key = key("foo" + i);
       assertTrue(nodeCreated.contains(key));
       assertTrue(valuesSet.contains(key));
-      assertThat(graph.get(key).getValue()).isEqualTo(new StringValue("bar" + i));
-      assertThat(graph.get(key).getVersion()).isEqualTo(startingVersion);
+      assertThat(graph.get(null, Reason.OTHER, key).getValue())
+          .isEqualTo(new StringValue("bar" + i));
+      assertThat(graph.get(null, Reason.OTHER, key).getVersion()).isEqualTo(startingVersion);
     }
   }
 
@@ -289,16 +291,16 @@ public abstract class GraphConcurrencyTest {
     for (int i = 0; i < numKeys; i++) {
       keys.add(key("foo" + i));
     }
-    Map<SkyKey, NodeEntry> entries = graph.createIfAbsentBatch(keys);
+    Map<SkyKey, NodeEntry> entries = graph.createIfAbsentBatch(null, Reason.OTHER, keys);
     for (int i = 0; i < numKeys; i++) {
       NodeEntry entry = entries.get(key("foo" + i));
       startEvaluation(entry);
       entry.setValue(new StringValue("bar"), startingVersion);
     }
 
-    assertNotNull(graph.get(key("foo" + 0)));
+    assertNotNull(graph.get(null, Reason.OTHER, key("foo" + 0)));
     graph = getGraph(getNextVersion(startingVersion));
-    assertNotNull(graph.get(key("foo" + 0)));
+    assertNotNull(graph.get(null, Reason.OTHER, key("foo" + 0)));
     ExecutorService pool1 = Executors.newFixedThreadPool(numThreads);
     ExecutorService pool2 = Executors.newFixedThreadPool(numThreads);
     ExecutorService pool3 = Executors.newFixedThreadPool(numThreads);
@@ -323,7 +325,7 @@ public abstract class GraphConcurrencyTest {
               } catch (InterruptedException e) {
                 throw new AssertionError(e);
               }
-              NodeEntry entry = graph.get(key("foo" + keyNum));
+              NodeEntry entry = graph.get(null, Reason.OTHER, key("foo" + keyNum));
               entry.markDirty(true);
               // Make some changes, like adding a dep and rdep.
               entry.addReverseDepAndCheckIfDone(key("rdep"));
@@ -345,7 +347,7 @@ public abstract class GraphConcurrencyTest {
               } catch (InterruptedException e) {
                 throw new AssertionError(e);
               }
-              NodeEntry entry = graph.get(key("foo" + keyNum));
+              NodeEntry entry = graph.get(null, Reason.OTHER, key("foo" + keyNum));
               assertNotNull(entry);
               // Requests for the value are made at the same time that the version increments from
               // the base. Check that there is no problem in requesting the version and that the
@@ -379,7 +381,7 @@ public abstract class GraphConcurrencyTest {
                 throw new AssertionError(e);
               }
               Map<SkyKey, NodeEntry> batchMap =
-                  graph.getBatchWithFieldHints(batch, NodeEntryField.NO_FIELDS);
+                  graph.getBatchWithFieldHints(null, Reason.OTHER, batch, NodeEntryField.NO_FIELDS);
               getBatchCountDownLatch.countDown();
               assertThat(batchMap).hasSize(batch.size());
               for (NodeEntry entry : batchMap.values()) {
@@ -398,7 +400,7 @@ public abstract class GraphConcurrencyTest {
     assertFalse(ExecutorUtil.interruptibleShutdown(pool2));
     assertFalse(ExecutorUtil.interruptibleShutdown(pool3));
     for (int i = 0; i < numKeys; i++) {
-      NodeEntry entry = graph.get(key("foo" + i));
+      NodeEntry entry = graph.get(null, Reason.OTHER, key("foo" + i));
       assertThat(entry.getValue()).isEqualTo(new StringValue("bar" + i));
       assertThat(entry.getVersion()).isEqualTo(getNextVersion(startingVersion));
       for (SkyKey key : entry.getReverseDeps()) {
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 711e9e7b2d..af0049b52b 100644
--- a/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java
+++ b/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java
@@ -19,7 +19,6 @@ 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 java.util.EnumSet;
 import java.util.Map;
 import java.util.Set;
@@ -89,8 +88,10 @@ public class NotifyingHelper {
     }
 
     @Override
-    public Map<SkyKey, NodeEntry> getBatch(Iterable<SkyKey> keys) {
-      return Maps.transformEntries(delegate.getBatch(keys), notifyingHelper.wrapEntry);
+    public Map<SkyKey, NodeEntry> getBatchForInvalidation(Iterable<SkyKey> keys) {
+      return Maps.transformEntries(
+          delegate.getBatchForInvalidation(keys),
+          notifyingHelper.wrapEntry);
     }
   }
 
@@ -114,24 +115,31 @@ public class NotifyingHelper {
     }
 
     @Override
-    public Map<SkyKey, NodeEntry> createIfAbsentBatch(Iterable<SkyKey> keys) {
+    public Map<SkyKey, NodeEntry> createIfAbsentBatch(
+        @Nullable SkyKey requestor, Reason reason, Iterable<SkyKey> keys) {
       for (SkyKey key : keys) {
         notifyingHelper.graphListener.accept(key, EventType.CREATE_IF_ABSENT, Order.BEFORE, null);
       }
-      return Maps.transformEntries(delegate.createIfAbsentBatch(keys), notifyingHelper.wrapEntry);
+      return Maps.transformEntries(
+          delegate.createIfAbsentBatch(requestor, reason, keys),
+          notifyingHelper.wrapEntry);
     }
 
     @Override
     public Map<SkyKey, NodeEntry> getBatchWithFieldHints(
-        Iterable<SkyKey> keys, EnumSet<NodeEntryField> fields) {
+        @Nullable SkyKey requestor,
+        Reason reason,
+        Iterable<SkyKey> keys,
+        EnumSet<NodeEntryField> fields) {
       return Maps.transformEntries(
-          delegate.getBatchWithFieldHints(keys, fields), notifyingHelper.wrapEntry);
+          delegate.getBatchWithFieldHints(requestor, reason, keys, fields),
+          notifyingHelper.wrapEntry);
     }
 
     @Nullable
     @Override
-    public NodeEntry get(SkyKey key) {
-      return notifyingHelper.wrapEntry(key, delegate.get(key));
+    public NodeEntry get(@Nullable SkyKey requestor, Reason reason, SkyKey key) {
+      return notifyingHelper.wrapEntry(key, delegate.get(requestor, reason, key));
     }
   }
 
diff --git a/src/test/java/com/google/devtools/build/skyframe/NotifyingInMemoryGraph.java b/src/test/java/com/google/devtools/build/skyframe/NotifyingInMemoryGraph.java
index 1689440b79..d6d3ab04c7 100644
--- a/src/test/java/com/google/devtools/build/skyframe/NotifyingInMemoryGraph.java
+++ b/src/test/java/com/google/devtools/build/skyframe/NotifyingInMemoryGraph.java
@@ -23,8 +23,8 @@ class NotifyingInMemoryGraph extends NotifyingHelper.NotifyingProcessableGraph
   }
 
   @Override
-  public Map<SkyKey, NodeEntry> getBatch(Iterable<SkyKey> keys) {
-    return getBatchWithFieldHints(keys, NodeEntryField.ALL_FIELDS);
+  public Map<SkyKey, NodeEntry> getBatchForInvalidation(Iterable<SkyKey> keys) {
+    return getBatchWithFieldHints(null, Reason.INVALIDATION, keys, NodeEntryField.ALL_FIELDS);
   }
 
   @Override