Convert evaluated tracking to take a lazy SkyValue
The EvaluationProgressReceiver#evaluated method took a SkyValue,
but that parameter was only used by some of its implementations,
and only under some conditions.
Outside of tests, the main users are SkyframeBuildView's
ConfiguredTargetValueInvalidationReceiver and SkyframeBuilder's
ExecutionProgressReceiver.
The former builds up a set of built ConfiguredTarget keys when the
SkyValue is non-null and the EvaluationState is BUILT, and so its
nullity check can live behind those two conditions.
The latter cares about builting up a set of ConfiguredTargets, and
raising events on the eventBus when a TARGET_COMPLETION or
ASPECT_COMPLETION value is evaluated and is non-null. The extraction
of these values can live behind the conditions that check the type of
the SkyKey.
By making the SkyValue parameter lazy, this change enforces that it's
only accessed under these conditions.
This CL introduces a semantic change that should be small in effect.
The SkyframeBuildView keeps track of a set, dirtiedConfiguredTargetKeys,
and ConfiguredTarget keys evaluated as CLEAN were removed from it if
they had a non-null value. With this change, ConfiguredTarget keys
evaluated as CLEAN get removed regardless of whether their values are
null or non-null. The set is used to determine whether artifact conflict
search has to be rerun, and the extra removals that result from this CL
could cause fewer artifact conflict searches to run, but the only
affected searches would be those that were caused by dirtied configured
target values in error all of which were subsequently marked as clean,
which is probably rare.
--
MOS_MIGRATED_REVID=101144655
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 9c2f799..8a46276 100644
--- a/src/test/java/com/google/devtools/build/skyframe/EagerInvalidatorTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/EagerInvalidatorTest.java
@@ -24,6 +24,7 @@
import static org.junit.Assert.fail;
import com.google.common.base.Preconditions;
+import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
@@ -174,7 +175,8 @@
}
@Override
- public void evaluated(SkyKey skyKey, SkyValue value, EvaluationState state) {
+ public void evaluated(SkyKey skyKey, Supplier<SkyValue> skyValueSupplier,
+ EvaluationState state) {
throw new UnsupportedOperationException();
}
};
@@ -210,7 +212,8 @@
}
@Override
- public void evaluated(SkyKey skyKey, SkyValue value, EvaluationState state) {
+ public void evaluated(SkyKey skyKey, Supplier<SkyValue> skyValueSupplier,
+ EvaluationState state) {
throw new UnsupportedOperationException();
}
};
@@ -248,7 +251,8 @@
}
@Override
- public void evaluated(SkyKey skyKey, SkyValue value, EvaluationState state) {
+ public void evaluated(SkyKey skyKey, Supplier<SkyValue> skyValueSupplier,
+ EvaluationState state) {
throw new UnsupportedOperationException();
}
};
@@ -358,7 +362,8 @@
}
@Override
- public void evaluated(SkyKey skyKey, SkyValue value, EvaluationState state) {
+ public void evaluated(SkyKey skyKey, Supplier<SkyValue> skyValueSupplier,
+ EvaluationState state) {
throw new UnsupportedOperationException();
}
};
@@ -386,7 +391,8 @@
}
@Override
- public void evaluated(SkyKey skyKey, SkyValue value, EvaluationState state) {
+ public void evaluated(SkyKey skyKey, Supplier<SkyValue> skyValueSupplier,
+ EvaluationState state) {
throw new UnsupportedOperationException();
}
};
@@ -484,7 +490,8 @@
}
@Override
- public void evaluated(SkyKey skyKey, SkyValue value, EvaluationState state) {
+ public void evaluated(SkyKey skyKey, Supplier<SkyValue> skyValueSupplier,
+ EvaluationState state) {
throw new UnsupportedOperationException();
}
};
diff --git a/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java b/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java
index d414fea..5d90a50 100644
--- a/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java
@@ -31,6 +31,7 @@
import static org.junit.Assert.fail;
import com.google.common.base.Predicate;
+import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -255,7 +256,8 @@
public void enqueueing(SkyKey key) {}
@Override
- public void evaluated(SkyKey skyKey, SkyValue value, EvaluationState state) {
+ public void evaluated(SkyKey skyKey, Supplier<SkyValue> skyValueSupplier,
+ EvaluationState state) {
receivedValues.add(skyKey);
}
};
@@ -1896,7 +1898,8 @@
}
@Override
- public void evaluated(SkyKey skyKey, SkyValue value, EvaluationState state) {
+ public void evaluated(SkyKey skyKey, Supplier<SkyValue> skyValueSupplier,
+ EvaluationState state) {
evaluatedValues.add(skyKey);
}
};
diff --git a/src/test/java/com/google/devtools/build/skyframe/TrackingInvalidationReceiver.java b/src/test/java/com/google/devtools/build/skyframe/TrackingInvalidationReceiver.java
index a9a7af3..090f04a 100644
--- a/src/test/java/com/google/devtools/build/skyframe/TrackingInvalidationReceiver.java
+++ b/src/test/java/com/google/devtools/build/skyframe/TrackingInvalidationReceiver.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.skyframe;
import com.google.common.base.Preconditions;
+import com.google.common.base.Supplier;
import com.google.common.collect.Sets;
import java.util.Set;
@@ -49,9 +50,9 @@
}
@Override
- public void evaluated(SkyKey skyKey, SkyValue value, EvaluationState state) {
+ public void evaluated(SkyKey skyKey, Supplier<SkyValue> skyValueSupplier, EvaluationState state) {
evaluated.add(skyKey);
- if (value != null) {
+ if (skyValueSupplier.get() != null) {
deleted.remove(skyKey);
if (state.equals(EvaluationState.CLEAN)) {
dirty.remove(skyKey);
