Add events and get rid of ErrorInfoEncoder. Clean up some signatures and visibility in Skyframe classes.
PiperOrigin-RevId: 197665817
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/DeserializationContext.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/DeserializationContext.java
index 45bde75..7a3dcae 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/DeserializationContext.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/DeserializationContext.java
@@ -106,7 +106,7 @@
* context.
*/
@CheckReturnValue
- DeserializationContext getMemoizingContext() {
+ public DeserializationContext getMemoizingContext() {
if (deserializer != null) {
return this;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationContext.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationContext.java
index c16c647..dedbf2d 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationContext.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationContext.java
@@ -102,7 +102,7 @@
* context.
*/
@CheckReturnValue
- SerializationContext getMemoizingContext() {
+ public SerializationContext getMemoizingContext() {
if (serializer != null) {
return this;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationResult.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationResult.java
index a22f546..90a1598 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationResult.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationResult.java
@@ -15,9 +15,7 @@
package com.google.devtools.build.lib.skyframe.serialization;
import com.google.common.base.Preconditions;
-import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
-import com.google.common.util.concurrent.MoreExecutors;
import com.google.protobuf.ByteString;
import javax.annotation.Nullable;
@@ -45,13 +43,6 @@
public abstract <S> SerializationResult<S> with(S newObj);
/**
- * Returns a new {@link SerializationResult} with the same object but with a new future that waits
- * on the given future as well as the current future.
- */
- public abstract SerializationResult<T> withAdditionalFuture(
- ListenableFuture<Void> additionalFuture);
-
- /**
* Returns a {@link ListenableFuture} that, if not null, must complete successfully before {@link
* #getObject} can be written remotely.
*/
@@ -86,11 +77,6 @@
}
@Override
- public SerializationResult<T> withAdditionalFuture(ListenableFuture<Void> additionalFuture) {
- return new ObjectWithFuture<>(getObject(), additionalFuture);
- }
-
- @Override
public ListenableFuture<Void> getFutureToBlockWritesOn() {
return null;
}
@@ -110,14 +96,6 @@
}
@Override
- public SerializationResult<T> withAdditionalFuture(ListenableFuture<Void> additionalFuture) {
- ListenableFuture<Void> combinedFuture =
- Futures.whenAllComplete(additionalFuture, futureToBlockWritesOn)
- .call(() -> null, MoreExecutors.directExecutor());
- return new ObjectWithFuture<>(getObject(), combinedFuture);
- }
-
- @Override
public ListenableFuture<Void> getFutureToBlockWritesOn() {
return futureToBlockWritesOn;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils/SerializationTester.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils/SerializationTester.java
index c8fda47..69433b1 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils/SerializationTester.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils/SerializationTester.java
@@ -58,7 +58,7 @@
void verifyDeserialized(T original, T deserialized) throws Exception;
}
- private final ImmutableList<Object> subjects;
+ private final ImmutableList<?> subjects;
private final ImmutableMap.Builder<Class<?>, Object> dependenciesBuilder;
private final ArrayList<ObjectCodec<?>> additionalCodecs = new ArrayList<>();
private boolean memoize;
@@ -75,7 +75,7 @@
this(ImmutableList.copyOf(subjects));
}
- public SerializationTester(ImmutableList<Object> subjects) {
+ public SerializationTester(ImmutableList<?> subjects) {
Preconditions.checkArgument(!subjects.isEmpty());
this.subjects = subjects;
this.dependenciesBuilder = ImmutableMap.builder();
diff --git a/src/main/java/com/google/devtools/build/skyframe/ErrorInfo.java b/src/main/java/com/google/devtools/build/skyframe/ErrorInfo.java
index 82d58ea..7a2c416 100644
--- a/src/main/java/com/google/devtools/build/skyframe/ErrorInfo.java
+++ b/src/main/java/com/google/devtools/build/skyframe/ErrorInfo.java
@@ -56,7 +56,7 @@
ImmutableList.of(cycleInfo),
/*isDirectlyTransient=*/ false,
/*isTransitivelyTransient=*/ false,
- /* isCatostrophic= */ false);
+ /* isCatastrophic= */ false);
}
/** Create an ErrorInfo from a collection of existing errors. */
@@ -110,7 +110,7 @@
ImmutableList<CycleInfo> cycles,
boolean isDirectlyTransient,
boolean isTransitivelyTransient,
- boolean isCatostrophic) {
+ boolean isCatastrophic) {
Preconditions.checkState(exception != null || !Iterables.isEmpty(cycles),
"At least one of exception and cycles must be non-null/empty, respectively");
Preconditions.checkState((exception == null) == (rootCauseOfException == null),
@@ -123,7 +123,7 @@
this.cycles = cycles;
this.isDirectlyTransient = isDirectlyTransient;
this.isTransitivelyTransient = isTransitivelyTransient;
- this.isCatastrophic = isCatostrophic;
+ this.isCatastrophic = isCatastrophic;
}
@Override
@@ -234,7 +234,7 @@
* path is returned here. However, if there are multiple paths to the same cycle, each of which
* goes through a different child, each of them is returned here.
*/
- public Iterable<CycleInfo> getCycleInfo() {
+ public ImmutableList<CycleInfo> getCycleInfo() {
return cycles;
}
diff --git a/src/main/java/com/google/devtools/build/skyframe/EventFilter.java b/src/main/java/com/google/devtools/build/skyframe/EventFilter.java
index 113e9f3..0b859d2 100644
--- a/src/main/java/com/google/devtools/build/skyframe/EventFilter.java
+++ b/src/main/java/com/google/devtools/build/skyframe/EventFilter.java
@@ -19,8 +19,8 @@
/** Filters out events which should not be stored during evaluation in {@link ParallelEvaluator}. */
public interface EventFilter extends Predicate<Event> {
/**
- * Returns true if any events should be stored. Otherwise, optimizations may be made to avoid
- * doing unnecessary work when evaluating node entries.
+ * Returns true if any events/postables should be stored. Otherwise, optimizations may be made to
+ * avoid doing unnecessary work when evaluating node entries.
*/
- boolean storeEvents();
+ boolean storeEventsAndPosts();
}
diff --git a/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java
index a035c03..a1f6461 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java
@@ -376,6 +376,7 @@
public ImmutableMap<SkyFunctionName, ? extends SkyFunction> getSkyFunctionsForTesting() {
return skyFunctions;
}
+
public static final EventFilter DEFAULT_STORED_EVENT_FILTER =
new EventFilter() {
@Override
@@ -390,7 +391,7 @@
}
@Override
- public boolean storeEvents() {
+ public boolean storeEventsAndPosts() {
return true;
}
};
diff --git a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java
index 920c6c1..905e822 100644
--- a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java
+++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java
@@ -24,6 +24,7 @@
import com.google.common.collect.Sets;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
@@ -190,6 +191,9 @@
NestedSet<TaggedEvents> buildEvents(NodeEntry entry, boolean missingChildren)
throws InterruptedException {
+ if (!evaluatorContext.getStoredEventFilter().storeEventsAndPosts()) {
+ return NestedSetBuilder.emptySet(Order.STABLE_ORDER);
+ }
// Aggregate the nested set of events from the direct deps, also adding the events from
// building this value.
NestedSetBuilder<TaggedEvents> eventBuilder = NestedSetBuilder.stableOrder();
@@ -197,29 +201,29 @@
if (!events.isEmpty()) {
eventBuilder.add(new TaggedEvents(getTagFromKey(), events));
}
- if (evaluatorContext.getStoredEventFilter().storeEvents()) {
- // Only do the work of processing children if we're going to store events.
- GroupedList<SkyKey> depKeys = entry.getTemporaryDirectDeps();
- Collection<SkyValue> deps = getDepValuesForDoneNodeMaybeFromError(depKeys);
- if (!missingChildren && depKeys.numElements() != deps.size()) {
- throw new IllegalStateException(
- "Missing keys for "
- + skyKey
- + ". Present values: "
- + deps
- + " requested from: "
- + depKeys
- + ", "
- + entry);
- }
- for (SkyValue value : deps) {
- eventBuilder.addTransitive(ValueWithMetadata.getEvents(value));
- }
+ GroupedList<SkyKey> depKeys = entry.getTemporaryDirectDeps();
+ Collection<SkyValue> deps = getDepValuesForDoneNodeMaybeFromError(depKeys);
+ if (!missingChildren && depKeys.numElements() != deps.size()) {
+ throw new IllegalStateException(
+ "Missing keys for "
+ + skyKey
+ + ". Present values: "
+ + deps
+ + " requested from: "
+ + depKeys
+ + ", "
+ + entry);
+ }
+ for (SkyValue value : deps) {
+ eventBuilder.addTransitive(ValueWithMetadata.getEvents(value));
}
return eventBuilder.build();
}
NestedSet<Postable> buildPosts(NodeEntry entry) throws InterruptedException {
+ if (!evaluatorContext.getStoredEventFilter().storeEventsAndPosts()) {
+ return NestedSetBuilder.emptySet(Order.STABLE_ORDER);
+ }
NestedSetBuilder<Postable> postBuilder = NestedSetBuilder.stableOrder();
postBuilder.addAll(eventHandler.getPosts());
diff --git a/src/main/java/com/google/devtools/build/skyframe/TaggedEvents.java b/src/main/java/com/google/devtools/build/skyframe/TaggedEvents.java
index 0d3e422..9c20763 100644
--- a/src/main/java/com/google/devtools/build/skyframe/TaggedEvents.java
+++ b/src/main/java/com/google/devtools/build/skyframe/TaggedEvents.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.skyframe;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.Collections2;
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
@@ -36,7 +37,8 @@
private final ImmutableList<Event> events;
private final int hashCode;
- TaggedEvents(final @Nullable String tag, ImmutableCollection<Event> events) {
+ @VisibleForTesting
+ public TaggedEvents(final @Nullable String tag, ImmutableCollection<Event> events) {
this.events =
events.isEmpty()
? ImmutableList.of()
diff --git a/src/main/java/com/google/devtools/build/skyframe/ValueWithMetadata.java b/src/main/java/com/google/devtools/build/skyframe/ValueWithMetadata.java
index ce92dfd..a0c5108 100644
--- a/src/main/java/com/google/devtools/build/skyframe/ValueWithMetadata.java
+++ b/src/main/java/com/google/devtools/build/skyframe/ValueWithMetadata.java
@@ -12,7 +12,10 @@
// See the License for the specific language governing permissions and
package com.google.devtools.build.skyframe;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.MoreObjects;
import com.google.common.base.Preconditions;
+import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
@@ -34,7 +37,7 @@
private static final NestedSet<Postable> NO_POSTS =
NestedSetBuilder.<Postable>emptySet(Order.STABLE_ORDER);
- public ValueWithMetadata(SkyValue value) {
+ private ValueWithMetadata(SkyValue value) {
this.value = value;
}
@@ -47,12 +50,12 @@
ErrorInfo errorInfo,
NestedSet<TaggedEvents> transitiveEvents,
NestedSet<Postable> transitivePostables) {
- return new ErrorInfoValue(errorInfo, null, transitiveEvents, transitivePostables);
+ return (ValueWithMetadata) normal(null, errorInfo, transitiveEvents, transitivePostables);
}
/**
- * Builds a value entry value that has a value value, and possibly an error (constructed from its
- * children's errors).
+ * Builds a SkyValue that has a value, and possibly an error, and possibly events/postables. If it
+ * has only a value, returns just the value in order to save memory.
*
* <p>This is public only for use in alternative {@code MemoizingEvaluator} implementations.
*/
@@ -83,8 +86,8 @@
public abstract NestedSet<Postable> getTransitivePostables();
/** Implementation of {@link ValueWithMetadata} for the value case. */
+ @VisibleForTesting
public static class ValueWithEvents extends ValueWithMetadata {
-
private final NestedSet<TaggedEvents> transitiveEvents;
private final NestedSet<Postable> transitivePostables;
@@ -97,7 +100,7 @@
this.transitivePostables = Preconditions.checkNotNull(transitivePostables);
}
- public static ValueWithEvents createValueWithEvents(
+ private static ValueWithEvents createValueWithEvents(
SkyValue value,
NestedSet<TaggedEvents> transitiveEvents,
NestedSet<Postable> transitivePostables) {
@@ -154,7 +157,13 @@
}
@Override
- public String toString() { return value.toString(); }
+ public String toString() {
+ return MoreObjects.toStringHelper(this)
+ .add("value", value)
+ .add("transitiveEvents size", Iterables.size(transitiveEvents))
+ .add("transitivePostables size", Iterables.size(transitivePostables))
+ .toString();
+ }
}
private static final class NotComparableValueWithEvents extends ValueWithEvents
