Convert invalidated tracking from per-value to per-key

The primary user of invalidation tracking is the SkyframeBuildView,
which monitored which ConfiguredTargetValues were invalidated. It
did that so the SkyframeExecutor could limit its search for artifact
conflicts to when the set of configured targets in the build changed.

For the newly introduced set of dirtied keys,
"dirtiedConfiguredTargetKeys" in SkyframeBuildView, to be as useful as
the "dirtyConfiguredTargets" set it replaced, the
ConfiguredTargetValueInvalidationReceiver must only remove a key from
the set if it was found to be clean when it was re-evaluated. If it
was rebuilt, then the key must stay in the set, to represent that the
set of configured target values has truly changed.

This CL introduces a semantic change that hopefully has a small effect,
if any. Previously, the informInvalidationReceiver method in
InvalidatingNodeVisitor only informed the invalidationReceiver when a
non-null value was invalidated. (This is usually, but not always,
equivalent to a non-error value. The uncommon exception is that in
keep-going builds, some nodes with errors may also have values, and
the invalidator would inform the receiver when such a node was
invalidated.) Now, the receiver is informed that a node is invalidated
regardless of whether its value is null. Because the receiver uses this
information to determine whether artifact conflict search has to be
rerun, and that search is expensive, it's possible this change will
negatively impact performance. However, the only way an extra search
could be invoked is if the invalidated configured target nodes are
all in error. That seems like it would happen rarely, if at all.

Further cleanup of unused SkyValues returned by markDirty to come in
a subsequent CL.

--
MOS_MIGRATED_REVID=100304744

1 files changed, 6 insertions, 8 deletions

diff --git a/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java b/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java
index 6ea61d68b0..e0d5b8c0e9 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java
@@ -121,10 +121,10 @@ public abstract class InvalidatingNodeVisitor extends AbstractQueueVisitor {
 
   protected abstract long count();
 
-  protected void informInvalidationReceiver(SkyValue value,
+  protected void informInvalidationReceiver(SkyKey key,
       EvaluationProgressReceiver.InvalidationState state) {
-    if (invalidationReceiver != null && value != null) {
-      invalidationReceiver.invalidated(value, state);
+    if (invalidationReceiver != null) {
+      invalidationReceiver.invalidated(key, state);
     }
   }
 
@@ -255,9 +255,8 @@ public abstract class InvalidatingNodeVisitor extends AbstractQueueVisitor {
                 }
               }
             }
-            // Allow custom Value-specific logic to update dirtiness status.
-            informInvalidationReceiver(entry.getValue(),
-                EvaluationProgressReceiver.InvalidationState.DELETED);
+            // Allow custom key-specific logic to update dirtiness status.
+            informInvalidationReceiver(key, EvaluationProgressReceiver.InvalidationState.DELETED);
           }
           if (traverseGraph) {
             // Force reverseDeps consolidation (validates that attempts to remove reverse deps were
@@ -380,8 +379,7 @@ public abstract class InvalidatingNodeVisitor extends AbstractQueueVisitor {
             child.removeReverseDep(key);
           }
 
-          SkyValue value = ValueWithMetadata.justValue(depsAndValue.second);
-          informInvalidationReceiver(value, EvaluationProgressReceiver.InvalidationState.DIRTY);
+          informInvalidationReceiver(key, EvaluationProgressReceiver.InvalidationState.DIRTY);
           dirtyKeyTracker.dirty(key);
           // Remove the node from the set as the last operation.
           pendingVisitations.remove(invalidationPair);