In AbstractSkyFunctionEnvironment, don't check for filtered exceptions in the common case of no exceptions. We were already mostly tracking missing dependencies in the subclasses, so there's no need to check for missing dependencies here.
PiperOrigin-RevId: 207934220
diff --git a/src/main/java/com/google/devtools/build/skyframe/AbstractSkyFunctionEnvironment.java b/src/main/java/com/google/devtools/build/skyframe/AbstractSkyFunctionEnvironment.java
index 712dc47..813dce7 100644
--- a/src/main/java/com/google/devtools/build/skyframe/AbstractSkyFunctionEnvironment.java
+++ b/src/main/java/com/google/devtools/build/skyframe/AbstractSkyFunctionEnvironment.java
@@ -28,6 +28,12 @@
@VisibleForTesting
public abstract class AbstractSkyFunctionEnvironment implements SkyFunction.Environment {
protected boolean valuesMissing = false;
+ // Hack for the common case that there are no errors in the retrieved values. In that case, we
+ // don't have to filter out any impermissible exceptions. Hack because we communicate this in an
+ // out-of-band way from #getValueOrUntypedExceptions. It's out-of-band because we don't want to
+ // incur the garbage overhead of returning a more complex data structure from
+ // #getValueOrUntypedExceptions.
+ protected boolean errorMightHaveBeenFound = false;
@Nullable private final GroupedList<SkyKey> temporaryDirectDeps;
public AbstractSkyFunctionEnvironment(@Nullable GroupedList<SkyKey> temporaryDirectDeps) {
@@ -134,7 +140,7 @@
public Map<SkyKey, SkyValue> getValues(Iterable<? extends SkyKey> depKeys)
throws InterruptedException {
Map<SkyKey, ValueOrUntypedException> valuesOrExceptions = getValueOrUntypedExceptions(depKeys);
- checkValuesAreMissing5(valuesOrExceptions, null, null, null, null, null);
+ checkValuesMissingBecauseOfFilteredError(valuesOrExceptions, null, null, null, null, null);
return Collections.unmodifiableMap(
Maps.transformValues(valuesOrExceptions, ValueOrUntypedException::getValue));
}
@@ -144,7 +150,8 @@
Iterable<? extends SkyKey> depKeys, Class<E> exceptionClass) throws InterruptedException {
SkyFunctionException.validateExceptionType(exceptionClass);
Map<SkyKey, ValueOrUntypedException> valuesOrExceptions = getValueOrUntypedExceptions(depKeys);
- checkValuesAreMissing5(valuesOrExceptions, exceptionClass, null, null, null, null);
+ checkValuesMissingBecauseOfFilteredError(
+ valuesOrExceptions, exceptionClass, null, null, null, null);
return Collections.unmodifiableMap(
Maps.transformValues(
valuesOrExceptions, voe -> ValueOrException.fromUntypedException(voe, exceptionClass)));
@@ -158,7 +165,8 @@
SkyFunctionException.validateExceptionType(exceptionClass1);
SkyFunctionException.validateExceptionType(exceptionClass2);
Map<SkyKey, ValueOrUntypedException> valuesOrExceptions = getValueOrUntypedExceptions(depKeys);
- checkValuesAreMissing5(valuesOrExceptions, exceptionClass1, exceptionClass2, null, null, null);
+ checkValuesMissingBecauseOfFilteredError(
+ valuesOrExceptions, exceptionClass1, exceptionClass2, null, null, null);
return Collections.unmodifiableMap(
Maps.transformValues(
valuesOrExceptions,
@@ -177,7 +185,7 @@
SkyFunctionException.validateExceptionType(exceptionClass2);
SkyFunctionException.validateExceptionType(exceptionClass3);
Map<SkyKey, ValueOrUntypedException> valuesOrExceptions = getValueOrUntypedExceptions(depKeys);
- checkValuesAreMissing5(
+ checkValuesMissingBecauseOfFilteredError(
valuesOrExceptions, exceptionClass1, exceptionClass2, exceptionClass3, null, null);
return Collections.unmodifiableMap(
Maps.transformValues(
@@ -201,7 +209,7 @@
SkyFunctionException.validateExceptionType(exceptionClass3);
SkyFunctionException.validateExceptionType(exceptionClass4);
Map<SkyKey, ValueOrUntypedException> valuesOrExceptions = getValueOrUntypedExceptions(depKeys);
- checkValuesAreMissing5(
+ checkValuesMissingBecauseOfFilteredError(
valuesOrExceptions,
exceptionClass1,
exceptionClass2,
@@ -237,7 +245,7 @@
SkyFunctionException.validateExceptionType(exceptionClass4);
SkyFunctionException.validateExceptionType(exceptionClass5);
Map<SkyKey, ValueOrUntypedException> valuesOrExceptions = getValueOrUntypedExceptions(depKeys);
- checkValuesAreMissing5(
+ checkValuesMissingBecauseOfFilteredError(
valuesOrExceptions,
exceptionClass1,
exceptionClass2,
@@ -263,13 +271,17 @@
E3 extends Exception,
E4 extends Exception,
E5 extends Exception>
- void checkValuesAreMissing5(
+ void checkValuesMissingBecauseOfFilteredError(
Map<SkyKey, ValueOrUntypedException> voes,
@Nullable Class<E1> exceptionClass1,
@Nullable Class<E2> exceptionClass2,
@Nullable Class<E3> exceptionClass3,
@Nullable Class<E4> exceptionClass4,
@Nullable Class<E5> exceptionClass5) {
+ if (!errorMightHaveBeenFound) {
+ // Short-circuit in the common case of no errors.
+ return;
+ }
for (ValueOrUntypedException voe : voes.values()) {
SkyValue value = voe.getValue();
if (value == null) {
diff --git a/src/main/java/com/google/devtools/build/skyframe/QueryableGraphBackedSkyFunctionEnvironment.java b/src/main/java/com/google/devtools/build/skyframe/QueryableGraphBackedSkyFunctionEnvironment.java
index 3be1c6a..3ac4e3d 100644
--- a/src/main/java/com/google/devtools/build/skyframe/QueryableGraphBackedSkyFunctionEnvironment.java
+++ b/src/main/java/com/google/devtools/build/skyframe/QueryableGraphBackedSkyFunctionEnvironment.java
@@ -35,9 +35,9 @@
this.eventHandler = eventHandler;
}
- private static ValueOrUntypedException toUntypedValue(NodeEntry nodeEntry)
- throws InterruptedException {
+ private ValueOrUntypedException toUntypedValue(NodeEntry nodeEntry) throws InterruptedException {
if (nodeEntry == null || !nodeEntry.isDone()) {
+ valuesMissing = true;
return ValueOrUntypedException.ofNull();
}
SkyValue maybeWrappedValue = nodeEntry.getValueMaybeWithMetadata();
@@ -45,6 +45,7 @@
if (justValue != null) {
return ValueOrUntypedException.ofValueUntyped(justValue);
}
+ errorMightHaveBeenFound = true;
ErrorInfo errorInfo =
Preconditions.checkNotNull(ValueWithMetadata.getMaybeErrorInfo(maybeWrappedValue));
Exception exception = errorInfo.getException();
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 c0048e7..b5fb94c 100644
--- a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java
+++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java
@@ -535,6 +535,7 @@
ErrorInfo errorInfo = ValueWithMetadata.getMaybeErrorInfo(depValue);
if (errorInfo != null) {
+ errorMightHaveBeenFound = true;
childErrorInfos.add(errorInfo);
if (bubbleErrorInfo != null) {
// Set interrupted status, to try to prevent the calling SkyFunction from doing anything
diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
index 013f57e..80b8e7e 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
@@ -233,6 +233,7 @@
result.put(key, ValueOrUntypedException.ofValueUntyped(value));
continue;
}
+ errorMightHaveBeenFound = true;
ErrorInfo errorInfo = evaluationResult.getError(key);
if (errorInfo == null || errorInfo.getException() == null) {
result.put(key, ValueOrUntypedException.ofNull());
