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

3 files changed, 23 insertions, 9 deletions

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 712dc4773a..813dce7df9 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 @@ import javax.annotation.Nullable;
 @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 abstract class AbstractSkyFunctionEnvironment implements SkyFunction.Envi
   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 @@ public abstract class AbstractSkyFunctionEnvironment implements SkyFunction.Envi
       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 @@ public abstract class AbstractSkyFunctionEnvironment implements SkyFunction.Envi
     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 @@ public abstract class AbstractSkyFunctionEnvironment implements SkyFunction.Envi
     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 @@ public abstract class AbstractSkyFunctionEnvironment implements SkyFunction.Envi
     SkyFunctionException.validateExceptionType(exceptionClass3);
     SkyFunctionException.validateExceptionType(exceptionClass4);
     Map<SkyKey, ValueOrUntypedException> valuesOrExceptions = getValueOrUntypedExceptions(depKeys);
-    checkValuesAreMissing5(
+    checkValuesMissingBecauseOfFilteredError(
         valuesOrExceptions,
         exceptionClass1,
         exceptionClass2,
@@ -237,7 +245,7 @@ public abstract class AbstractSkyFunctionEnvironment implements SkyFunction.Envi
     SkyFunctionException.validateExceptionType(exceptionClass4);
     SkyFunctionException.validateExceptionType(exceptionClass5);
     Map<SkyKey, ValueOrUntypedException> valuesOrExceptions = getValueOrUntypedExceptions(depKeys);
-    checkValuesAreMissing5(
+    checkValuesMissingBecauseOfFilteredError(
         valuesOrExceptions,
         exceptionClass1,
         exceptionClass2,
@@ -263,13 +271,17 @@ public abstract class AbstractSkyFunctionEnvironment implements SkyFunction.Envi
           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 3be1c6a418..3ac4e3d7eb 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 @@ public class QueryableGraphBackedSkyFunctionEnvironment extends AbstractSkyFunct
     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 @@ public class QueryableGraphBackedSkyFunctionEnvironment extends AbstractSkyFunct
     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 c0048e7587..b5fb94c751 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 @@ class SkyFunctionEnvironment extends AbstractSkyFunctionEnvironment {
 
       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