Automated rollback of commit 7fe59b98eefc96a6310f0b0221d4e0f18e2a9000. *** Reason for rollback *** Fixed bug due to TransitiveTargetFunction requesting multiple Package dependencies when computing its aspect deps by only applying the optimization to TransitiveTraversalFunction. *** Original change description *** Automated rollback of commit cce164aed44aba1de244f0d764cd33a5cc6980b2. PiperOrigin-RevId: 186766812

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveBaseTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveBaseTraversalFunction.java
index b51bab4..e438d91 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveBaseTraversalFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveBaseTraversalFunction.java

@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.skyframe;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.Collections2;
 import com.google.common.collect.ImmutableSet;
@@ -61,7 +62,6 @@
  * return.
  */
 abstract class TransitiveBaseTraversalFunction<TProcessedTargets> implements SkyFunction {
-
   /**
    * Returns a {@link SkyKey} corresponding to the traversal of a target specified by {@code label}
    * and its transitive dependencies.
@@ -126,9 +126,9 @@
         loadTargetResultsType);
     TargetAndErrorIfAny targetAndErrorIfAny = (TargetAndErrorIfAny) loadTargetResults;
 
-    // Process deps from attributes.
-    Collection<SkyKey> labelDepKeys =
-        Collections2.transform(getLabelDeps(targetAndErrorIfAny.getTarget()), this::getKey);
+    // Process deps from attributes. It is essential that the last getValue(s) call we made to
+    // skyframe for building this node was for the corresponding PackageValue.
+    Collection<SkyKey> labelDepKeys = getLabelDepKeys(env, targetAndErrorIfAny);
 
     Map<SkyKey, ValueOrException2<NoSuchPackageException, NoSuchTargetException>> depMap =
         env.getValuesOrThrow(labelDepKeys, NoSuchPackageException.class,
@@ -136,9 +136,10 @@
     if (env.valuesMissing()) {
       return null;
     }
-    // Process deps from aspects.
+    // Process deps from attributes. It is essential that the second-to-last getValue(s) call we
+    // made to skyframe for building this node was for the corresponding PackageValue.
     Iterable<SkyKey> labelAspectKeys =
-        getStrictLabelAspectKeys(targetAndErrorIfAny.getTarget(), depMap, env);
+        getStrictLabelAspectDepKeys(env, depMap, targetAndErrorIfAny);
     Set<Entry<SkyKey, ValueOrException2<NoSuchPackageException, NoSuchTargetException>>>
         labelAspectEntries = env.getValuesOrThrow(labelAspectKeys, NoSuchPackageException.class,
         NoSuchTargetException.class).entrySet();
@@ -153,6 +154,20 @@
     return computeSkyValue(targetAndErrorIfAny, processedTargets);
   }
 
+  Collection<SkyKey> getLabelDepKeys(
+      SkyFunction.Environment env, TargetAndErrorIfAny targetAndErrorIfAny)
+      throws InterruptedException {
+    return Collections2.transform(getLabelDeps(targetAndErrorIfAny.getTarget()), this::getKey);
+  }
+
+  Iterable<SkyKey> getStrictLabelAspectDepKeys(
+      SkyFunction.Environment env,
+      Map<SkyKey, ValueOrException2<NoSuchPackageException, NoSuchTargetException>> depMap,
+      TargetAndErrorIfAny targetAndErrorIfAny)
+      throws InterruptedException {
+    return getStrictLabelAspectKeys(targetAndErrorIfAny.getTarget(), depMap, env);
+  }
+
   @Override
   public String extractTag(SkyKey skyKey) {
     return Label.print(argumentFromKey(skyKey));
@@ -269,14 +284,18 @@
     Target getTarget();
   }
 
-  private static class TargetAndErrorIfAnyImpl implements TargetAndErrorIfAny, LoadTargetResults {
+  @VisibleForTesting
+  static class TargetAndErrorIfAnyImpl implements TargetAndErrorIfAny, LoadTargetResults {
 
     private final boolean packageLoadedSuccessfully;
     @Nullable private final NoSuchTargetException errorLoadingTarget;
     private final Target target;
 
-    private TargetAndErrorIfAnyImpl(boolean packageLoadedSuccessfully,
-        @Nullable NoSuchTargetException errorLoadingTarget, Target target) {
+    @VisibleForTesting
+    TargetAndErrorIfAnyImpl(
+        boolean packageLoadedSuccessfully,
+        @Nullable NoSuchTargetException errorLoadingTarget,
+        Target target) {
       this.packageLoadedSuccessfully = packageLoadedSuccessfully;
       this.errorLoadingTarget = errorLoadingTarget;
       this.target = target;
@@ -304,7 +323,7 @@
     }
   }
 
-  private LoadTargetResults loadTarget(Environment env, Label label)
+  LoadTargetResults loadTarget(Environment env, Label label)
       throws NoSuchTargetException, NoSuchPackageException, InterruptedException {
     SkyKey packageKey = PackageValue.key(label.getPackageIdentifier());
     SkyKey targetKey = TargetMarkerValue.key(label);

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java
index a537378..eb7bd63 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java

@@ -26,10 +26,14 @@
 import com.google.devtools.build.lib.packages.NoSuchThingException;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.skyframe.TransitiveTraversalFunction.FirstErrorMessageAccumulator;
+import com.google.devtools.build.lib.util.GroupedList;
+import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
 import com.google.devtools.build.skyframe.ValueOrException2;
 import java.util.Collection;
+import java.util.List;
+import java.util.Map;
 import java.util.Map.Entry;
 import javax.annotation.Nullable;
 
@@ -130,6 +134,63 @@
     return TargetMarkerFunction.computeTargetMarkerValue(targetMarkerKey, env);
   }
 
+  @Override
+  Collection<SkyKey> getLabelDepKeys(
+      SkyFunction.Environment env, TargetAndErrorIfAny targetAndErrorIfAny)
+      throws InterruptedException {
+    // As a performance optimization we may already know the deps we are  about to request from
+    // last time #compute was called. By requesting these from the environment, we can avoid
+    // repeating the label visitation step. For TransitiveTraversalFunction#compute, the label deps
+    // dependency group is requested immediately after the package.
+    //
+    // IMPORTANT: No other package values should be requested inside
+    // TransitiveTraversalFunction#compute from this point forward.
+    Collection<SkyKey> oldDepKeys = getDepsAfterLastPackageDep(env, /*offset=*/ 1);
+    return oldDepKeys == null ? super.getLabelDepKeys(env, targetAndErrorIfAny) : oldDepKeys;
+  }
+
+  @Override
+  Iterable<SkyKey> getStrictLabelAspectDepKeys(
+      SkyFunction.Environment env,
+      Map<SkyKey, ValueOrException2<NoSuchPackageException, NoSuchTargetException>> depMap,
+      TargetAndErrorIfAny targetAndErrorIfAny)
+      throws InterruptedException {
+    // As a performance optimization we may already know the deps we are  about to request from
+    // last time #compute was called. By requesting these from the environment, we can avoid
+    // repeating the label visitation step. For TransitiveTraversalFunction#compute, the label
+    // aspect deps dependency group is requested two groups after the package.
+    Collection<SkyKey> oldAspectDepKeys = getDepsAfterLastPackageDep(env, /*offset=*/ 2);
+    return oldAspectDepKeys == null
+        ? super.getStrictLabelAspectDepKeys(env, depMap, targetAndErrorIfAny)
+        : oldAspectDepKeys;
+  }
+
+  @Nullable
+  private static Collection<SkyKey> getDepsAfterLastPackageDep(
+      SkyFunction.Environment env, int offset) {
+    GroupedList<SkyKey> temporaryDirectDeps = env.getTemporaryDirectDeps();
+    if (temporaryDirectDeps == null) {
+      return null;
+    }
+    int lastPackageDepIndex = getLastPackageValueIndex(temporaryDirectDeps);
+    if (lastPackageDepIndex == -1
+        || temporaryDirectDeps.listSize() <= lastPackageDepIndex + offset) {
+      return null;
+    }
+    return temporaryDirectDeps.get(lastPackageDepIndex + offset);
+  }
+
+  private static int getLastPackageValueIndex(GroupedList<SkyKey> directDeps) {
+    int directDepsNumGroups = directDeps.listSize();
+    for (int i = directDepsNumGroups - 1; i >= 0; i--) {
+      List<SkyKey> depGroup = directDeps.get(i);
+      if (depGroup.size() == 1 && depGroup.get(0).functionName().equals(SkyFunctions.PACKAGE)) {
+        return i;
+      }
+    }
+    return -1;
+  }
+
   /**
    * Keeps track of the first error message encountered while traversing itself and its
    * dependencies.

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 7c5deab..2b0984d 100644
--- a/src/main/java/com/google/devtools/build/skyframe/AbstractSkyFunctionEnvironment.java
+++ b/src/main/java/com/google/devtools/build/skyframe/AbstractSkyFunctionEnvironment.java

@@ -17,6 +17,7 @@
 import com.google.common.base.Function;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Maps;
+import com.google.devtools.build.lib.util.GroupedList;
 import com.google.devtools.build.skyframe.ValueOrExceptionUtils.BottomException;
 import java.util.Collections;
 import java.util.Map;
@@ -29,12 +30,27 @@
 @VisibleForTesting
 public abstract class AbstractSkyFunctionEnvironment implements SkyFunction.Environment {
   protected boolean valuesMissing = false;
+  @Nullable private final GroupedList<SkyKey> temporaryDirectDeps;
+
   private <E extends Exception> ValueOrException<E> getValueOrException(
       SkyKey depKey, Class<E> exceptionClass) throws InterruptedException {
     return ValueOrExceptionUtils.downconvert(
         getValueOrException(depKey, exceptionClass, BottomException.class), exceptionClass);
   }
 
+  public AbstractSkyFunctionEnvironment(@Nullable GroupedList<SkyKey> temporaryDirectDeps) {
+    this.temporaryDirectDeps = temporaryDirectDeps;
+  }
+
+  public AbstractSkyFunctionEnvironment() {
+    this(null);
+  }
+
+  @Override
+  public GroupedList<SkyKey> getTemporaryDirectDeps() {
+    return temporaryDirectDeps;
+  }
+
   private <E1 extends Exception, E2 extends Exception>
       ValueOrException2<E1, E2> getValueOrException(
           SkyKey depKey, Class<E1> exceptionClass1, Class<E2> exceptionClass2)

diff --git a/src/main/java/com/google/devtools/build/skyframe/SkyFunction.java b/src/main/java/com/google/devtools/build/skyframe/SkyFunction.java
index 66e9408..9b7c29d 100644
--- a/src/main/java/com/google/devtools/build/skyframe/SkyFunction.java
+++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunction.java

@@ -16,6 +16,7 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
+import com.google.devtools.build.lib.util.GroupedList;
 import java.util.Map;
 import javax.annotation.Nullable;
 
@@ -273,6 +274,16 @@
      */
     ExtendedEventHandler getListener();
 
+    /**
+     * A live view of deps known to have already been requested either through an earlier call to
+     * {@link SkyFunction#compute} or inferred during change pruning. Should return {@code null} if
+     * unknown.
+     */
+    @Nullable
+    default GroupedList<SkyKey> getTemporaryDirectDeps() {
+      return null;
+    }
+
     /** Returns whether we are currently in error bubbling. */
     @VisibleForTesting
     boolean inErrorBubblingForTesting();

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 91767cc..9646b07 100644
--- a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java
+++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java

@@ -129,6 +129,7 @@
       Set<SkyKey> oldDeps,
       ParallelEvaluatorContext evaluatorContext)
       throws InterruptedException {
+    super(directDeps);
     this.skyKey = skyKey;
     this.oldDeps = oldDeps;
     this.evaluatorContext = evaluatorContext;