Automated rollback of commit 6224c87.

*** Reason for rollback ***

No longer needed, change is not good for walltime under prioritization.

This change was originally needed to ensure prioritization information
could propagate, as it had been held within SkyKeys. This caused a ~10MB
heap-after-gc regression. The new approach propagates priority information
through DirtyBuildingState which avoids this memory regression because the
state is cleaned up after evaluation.

With prioritization, there are 2 queues, a priority queue and a FIFO. Going
through the priority queue is much more expensive than the FIFO. So it's better
to not mark too many keys CPUHeavy.

Additionally, requiring the keys to be transitively connected would be a very
difficult property to maintain over time.

*** Original change description ***

Mark SkyKeys having CPUHeavy dependencies CPUHeavy.

* This may improve walltime in high CPU settings.
* This is a precursor to SkyKey prioritization, which requires it.

PiperOrigin-RevId: 509253113
Change-Id: I55479c3196c96c5e9319da7e052cb153cd24229f

src/main/java/com/google/devtools/build/lib/actions/ActionLookupKey.java

@@ -30,14 +30,14 @@
  * are subclasses of {@link ActionLookupKey}. This allows callers to easily find the value key,
  * while remaining agnostic to what action lookup values actually exist.
  */
-public abstract class ActionLookupKey extends CPUHeavySkyKey implements ArtifactOwner {
+public interface ActionLookupKey extends ArtifactOwner, CPUHeavySkyKey {
 
   /**
    * Returns the {@link BuildConfigurationKey} for the configuration associated with this key, or
    * {@code null} if this key has no associated configuration.
    */
   @Nullable
-  public abstract BuildConfigurationKey getConfigurationKey();
+  BuildConfigurationKey getConfigurationKey();
 
   /**
    * Returns {@code true} if this key <em>may</em> own shareable actions, as determined by {@link
@@ -50,7 +50,7 @@ public abstract class ActionLookupKey extends CPUHeavySkyKey implements Artifact
    * to determine whether the individual action can be shared - notably, for a test target,
    * compilation actions are shareable, but test actions are not.
    */
-  public boolean mayOwnShareableActions() {
+  default boolean mayOwnShareableActions() {
     return getLabel() != null;
   }
 }

src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java

@@ -36,7 +36,7 @@ public static ActionTemplateExpansionKey key(ActionLookupKey actionLookupKey, in
 
   /** Key for {@link ActionTemplateExpansionValue} nodes. */
   @AutoCodec
-  public static final class ActionTemplateExpansionKey extends ActionLookupKey {
+  public static final class ActionTemplateExpansionKey implements ActionLookupKey {
     private static final Interner<ActionTemplateExpansionKey> interner =
         BlazeInterners.newWeakInterner();
 

src/main/java/com/google/devtools/build/lib/skyframe/AspectKeyCreator.java

@@ -60,7 +60,7 @@ public static TopLevelAspectsKey createTopLevelAspectsKey(
   }
 
   /** Common superclass for {@link AspectKey} and {@link TopLevelAspectsKey}. */
-  public abstract static class AspectBaseKey extends ActionLookupKey {
+  public abstract static class AspectBaseKey implements ActionLookupKey {
     private final ConfiguredTargetKey baseConfiguredTargetKey;
     private final int hashCode;
 

src/main/java/com/google/devtools/build/lib/skyframe/BUILD

@@ -346,7 +346,6 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/vfs:output_service",
         "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
         "//src/main/java/com/google/devtools/build/skyframe",
-        "//src/main/java/com/google/devtools/build/skyframe:cpu_heavy_skykey",
         "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
         "//src/main/java/com/google/devtools/common/options",
         "//src/main/java/net/starlark/java/eval",
@@ -2242,7 +2241,6 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
         "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
         "//src/main/java/com/google/devtools/build/lib/util:exit_code",
-        "//src/main/java/com/google/devtools/build/skyframe:cpu_heavy_skykey",
         "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
         "//third_party:auto_value",
         "//third_party:guava",
@@ -2416,7 +2414,6 @@ java_library(
         ":sky_functions",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant",
-        "//src/main/java/com/google/devtools/build/skyframe:cpu_heavy_skykey",
         "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
         "//src/main/java/net/starlark/java/eval",
         "//third_party:guava",
@@ -2435,7 +2432,6 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/packages",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
         "//src/main/java/com/google/devtools/build/lib/vfs",
-        "//src/main/java/com/google/devtools/build/skyframe:cpu_heavy_skykey",
         "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
         "//src/main/java/net/starlark/java/eval",
         "//third_party:guava",
@@ -2637,7 +2633,6 @@ java_library(
         ":sky_functions",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/toolchain_type_requirement",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
-        "//src/main/java/com/google/devtools/build/skyframe:cpu_heavy_skykey",
         "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
         "//third_party:auto_value",
         "//third_party:guava",

src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverKey.java

@@ -23,7 +23,7 @@
  * Wraps an {@link ActionLookupKey}. The evaluation of this SkyKey is the entry point of analyzing
  * the {@link ActionLookupKey} and executing the associated actions.
  */
-public final class BuildDriverKey extends CPUHeavySkyKey {
+public final class BuildDriverKey implements CPUHeavySkyKey {
   private final ActionLookupKey actionLookupKey;
   private final TopLevelArtifactContext topLevelArtifactContext;
   private final TestType testType;

src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionValue.java

@@ -57,7 +57,7 @@ public static BuildInfoKeyAndConfig key(
 
   /** Key for BuildInfoCollectionValues. */
   @AutoCodec
-  public static final class BuildInfoKeyAndConfig extends ActionLookupKey {
+  public static final class BuildInfoKeyAndConfig implements ActionLookupKey {
     private static final Interner<BuildInfoKeyAndConfig> keyInterner =
         BlazeInterners.newWeakInterner();
 

src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadValue.java

@@ -25,8 +25,8 @@
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.vfs.Root;
 import com.google.devtools.build.lib.vfs.RootedPath;
-import com.google.devtools.build.skyframe.CPUHeavySkyKey;
 import com.google.devtools.build.skyframe.SkyFunctionName;
+import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
 import java.util.Objects;
 import net.starlark.java.eval.Module;
@@ -73,7 +73,7 @@ public BzlVisibility getBzlVisibility() {
   private static final Interner<Key> keyInterner = BlazeInterners.newWeakInterner();
 
   /** SkyKey for a Starlark load. */
-  public abstract static class Key extends CPUHeavySkyKey {
+  public abstract static class Key implements SkyKey {
     // Closed, for class-based equals()/hashCode().
     private Key() {}
 

src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java

@@ -58,7 +58,7 @@
  * BuildConfigurationKey.
  */
 @AutoCodec
-public class ConfiguredTargetKey extends ActionLookupKey {
+public class ConfiguredTargetKey implements ActionLookupKey {
   /**
    * Cache so that the number of ConfiguredTargetKey instances is {@code O(configured targets)} and
    * not {@code O(edges between configured targets)}.
@@ -101,7 +101,6 @@ public final SkyFunctionName functionName() {
   }
 
   @Nullable
-  @Override
   public final BuildConfigurationKey getConfigurationKey() {
     return configurationKey;
   }

src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportValue.java

@@ -32,7 +32,7 @@ public final class CoverageReportValue extends BasicActionLookupValue {
     super(generatingActions);
   }
 
-  private static final class CoverageReportKey extends ActionLookupKey {
+  private static final class CoverageReportKey implements ActionLookupKey {
     private CoverageReportKey() {}
 
     @Override

src/main/java/com/google/devtools/build/lib/skyframe/PackageValue.java

@@ -13,7 +13,6 @@
 // limitations under the License.
 package com.google.devtools.build.lib.skyframe;
 
-
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Interner;
@@ -23,6 +22,7 @@
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
 import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
+import com.google.devtools.build.skyframe.AbstractSkyKey;
 import com.google.devtools.build.skyframe.CPUHeavySkyKey;
 import com.google.devtools.build.skyframe.NotComparableSkyValue;
 import com.google.devtools.build.skyframe.SkyFunctionName;
@@ -63,18 +63,11 @@ public static Key key(PackageIdentifier pkgIdentifier) {
   /** Skyframe key for packages */
   @AutoCodec.VisibleForSerialization
   @AutoCodec
-  public static class Key extends CPUHeavySkyKey {
+  public static class Key extends AbstractSkyKey<PackageIdentifier> implements CPUHeavySkyKey {
     private static final Interner<Key> interner = BlazeInterners.newWeakInterner();
 
-    private final PackageIdentifier arg;
-
     private Key(PackageIdentifier arg) {
-      this.arg = arg;
-    }
-
-    @Override
-    public PackageIdentifier argument() {
-      return arg;
+      super(arg);
     }
 
     @AutoCodec.VisibleForSerialization
@@ -87,25 +80,6 @@ static Key create(PackageIdentifier arg) {
     public SkyFunctionName functionName() {
       return SkyFunctions.PACKAGE;
     }
-
-    @Override
-    public int hashCode() {
-      return 31 * functionName().hashCode() + arg.hashCode();
-    }
-
-    @Override
-    public boolean equals(Object obj) {
-      if (!(obj instanceof Key)) {
-        return false;
-      }
-      var that = (Key) obj;
-      return this.arg.equals(that.arg);
-    }
-
-    @Override
-    public String toString() {
-      return functionName() + ":" + arg;
-    }
   }
 
   public static ImmutableList<SkyKey> keys(Iterable<PackageIdentifier> pkgIdentifiers) {

src/main/java/com/google/devtools/build/lib/skyframe/RegisteredExecutionPlatformsValue.java

@@ -19,7 +19,6 @@
 import com.google.common.collect.Interner;
 import com.google.devtools.build.lib.concurrent.BlazeInterners;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
-import com.google.devtools.build.skyframe.CPUHeavySkyKey;
 import com.google.devtools.build.skyframe.SkyFunctionName;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
@@ -39,7 +38,7 @@ public static SkyKey key(BuildConfigurationKey configurationKey) {
   /** {@link SkyKey} implementation used for {@link RegisteredExecutionPlatformsFunction}. */
   @AutoCodec
   @AutoCodec.VisibleForSerialization
-  static class Key extends CPUHeavySkyKey {
+  static class Key implements SkyKey {
     private static final Interner<Key> interners = BlazeInterners.newWeakInterner();
 
     private final BuildConfigurationKey configurationKey;

src/main/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsValue.java

@@ -20,7 +20,6 @@
 import com.google.devtools.build.lib.analysis.platform.DeclaredToolchainInfo;
 import com.google.devtools.build.lib.concurrent.BlazeInterners;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
-import com.google.devtools.build.skyframe.CPUHeavySkyKey;
 import com.google.devtools.build.skyframe.SkyFunctionName;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
@@ -37,13 +36,9 @@ public static Key key(BuildConfigurationKey configurationKey) {
     return Key.of(configurationKey);
   }
 
-  /**
-   * A {@link SkyKey} for {@code RegisteredToolchainsValue}.
-   *
-   * <p>This is marked CPU-Heavy because it causes package loading.
-   */
+  /** A {@link SkyKey} for {@code RegisteredToolchainsValue}. */
   @AutoCodec
-  static class Key extends CPUHeavySkyKey {
+  static class Key implements SkyKey {
     private static final Interner<Key> interners = BlazeInterners.newWeakInterner();
 
     private final BuildConfigurationKey configurationKey;

src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingValue.java

@@ -23,7 +23,6 @@
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.util.DetailedExitCode;
 import com.google.devtools.build.lib.util.ExitCode;
-import com.google.devtools.build.skyframe.CPUHeavySkyKey;
 import com.google.devtools.build.skyframe.SkyFunctionName;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
@@ -100,13 +99,9 @@ public String toString() {
     return repositoryMapping.toString();
   }
 
-  /**
-   * {@link SkyKey} for {@link RepositoryMappingValue}.
-   *
-   * <p>Marked CPU heavy because it causes package loading.
-   */
+  /** {@link SkyKey} for {@link RepositoryMappingValue}. */
   @AutoValue
-  public abstract static class Key extends CPUHeavySkyKey {
+  public abstract static class Key implements SkyKey {
 
     private static final Interner<Key> interner = BlazeInterners.newWeakInterner();
 

src/main/java/com/google/devtools/build/lib/skyframe/SingleToolchainResolutionValue.java

@@ -21,7 +21,6 @@
 import com.google.devtools.build.lib.analysis.config.ToolchainTypeRequirement;
 import com.google.devtools.build.lib.analysis.platform.ToolchainTypeInfo;
 import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.skyframe.CPUHeavySkyKey;
 import com.google.devtools.build.skyframe.SkyFunctionName;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
@@ -66,13 +65,9 @@ public static SingleToolchainResolutionKey key(
         debugTarget);
   }
 
-  /**
-   * {@link SkyKey} implementation used for {@link SingleToolchainResolutionFunction}.
-   *
-   * <p>Marked CPU heavy because it transitively causes package loading.
-   */
+  /** {@link SkyKey} implementation used for {@link SingleToolchainResolutionFunction}. */
   @AutoValue
-  public abstract static class SingleToolchainResolutionKey extends CPUHeavySkyKey {
+  public abstract static class SingleToolchainResolutionKey implements SkyKey {
 
     @Override
     public SkyFunctionName functionName() {

src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsValue.java

@@ -17,8 +17,8 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;
-import com.google.devtools.build.skyframe.CPUHeavySkyKey;
 import com.google.devtools.build.skyframe.SkyFunctionName;
+import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
 import net.starlark.java.eval.StarlarkSemantics;
 
@@ -132,10 +132,8 @@ public static Key key() {
    * Skyframe key for retrieving the {@code @_builtins} definitions.
    *
    * <p>This has no fields since there is only one {@code StarlarkBuiltinsValue} at a time.
-   *
-   * <p>It is marked CPU heavy because it causes package loading.
    */
-  static final class Key extends CPUHeavySkyKey {
+  static final class Key implements SkyKey {
 
     private static final Key INSTANCE = new Key();
 

src/main/java/com/google/devtools/build/lib/skyframe/ToolchainContextKey.java

@@ -19,19 +19,17 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.analysis.config.ToolchainTypeRequirement;
 import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.skyframe.CPUHeavySkyKey;
 import com.google.devtools.build.skyframe.SkyFunctionName;
+import com.google.devtools.build.skyframe.SkyKey;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import java.util.Optional;
 
 /**
  * {@link SkyKey} implementation used for {@link ToolchainResolutionFunction} to produce {@link
  * UnloadedToolchainContextImpl} instances.
- *
- * <p>Marked CPU heavy because it may cause package loading.
  */
 @AutoValue
-public abstract class ToolchainContextKey extends CPUHeavySkyKey {
+public abstract class ToolchainContextKey implements SkyKey {
 
   /** Returns a new {@link Builder}. */
   public static Builder key() {

src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusValue.java

@@ -54,7 +54,7 @@ public Artifact getVolatileArtifact() {
   }
 
   /** {@link com.google.devtools.build.skyframe.SkyKey} for {@link WorkspaceStatusValue}. */
-  public static final class BuildInfoKey extends ActionLookupKey {
+  public static final class BuildInfoKey implements ActionLookupKey {
     private BuildInfoKey() {}
 
     @Override

src/main/java/com/google/devtools/build/skyframe/CPUHeavySkyKey.java

@@ -14,11 +14,9 @@
 package com.google.devtools.build.skyframe;
 
 /**
- * A {@link SkyKey} for a {@link SkyFunction} that causes heavy resource consumption.
- *
- * <p>This applies to both {@link SkyKey}s that have a high resource footprint and ancestors of
- * those {@link SkyKey}s that depend on them, transitively.
+ * An empty interface used to annotate whether the evaluation of a SkyKey contributes significantly
+ * to the CPU footprint of Skyframe.
  *
  * <p>This is currently only applicable to the loading/analysis phase of Skyframe.
  */
-public abstract class CPUHeavySkyKey implements SkyKey {}
+public interface CPUHeavySkyKey extends SkyKey {}

src/test/java/com/google/devtools/build/lib/actions/util/InjectedActionLookupKey.java

@@ -24,7 +24,7 @@
  * An {@link ActionLookupKey} with a non-hermetic {@link SkyFunctionName} so that its value can be
  * directly injected during tests.
  */
-public final class InjectedActionLookupKey extends ActionLookupKey {
+public final class InjectedActionLookupKey implements ActionLookupKey {
   public static final SkyFunctionName INJECTED_ACTION_LOOKUP =
       SkyFunctionName.createNonHermetic("INJECTED_ACTION_LOOKUP");
 

src/test/java/com/google/devtools/build/lib/analysis/RunfilesTest.java

@@ -155,7 +155,7 @@ private void checkConflictError() {
     assertThat(Iterables.getOnlyElement(eventCollector).getKind()).isEqualTo(EventKind.ERROR);
   }
 
-  private static final class SimpleActionLookupKey extends ActionLookupKey {
+  private static final class SimpleActionLookupKey implements ActionLookupKey {
     private final String name;
 
     SimpleActionLookupKey(String name) {