Subclass ActionOwner into two classes so as to save some memory

From statistics, most of the `ActionOwner` instances contain empty `aspectDescriptors` and `execProperties` fields (~88.4%). So I decided that we can implement two `ActionOwner` subclasses.

The `SimpleActionOwner` class does not contain these two fields, while `ComplexActionOwner` class extends `SimpleActionOwner` to include them.

For `SimpleActionOwner`, 8 bytes memory is saved (56->48) for each instance created.

PiperOrigin-RevId: 525795022
Change-Id: I48a964249f4cd1e6cf6b136e2b8bb68ba8c9c446

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

@@ -137,7 +137,7 @@ public Collection<BuildEventId> getChildrenEvents() {
   @Override
   public Collection<BuildEvent> getConfigurations() {
     if (action.getOwner() != null) {
-      BuildEvent configuration = action.getOwner().getConfiguration();
+      BuildEvent configuration = action.getOwner().getBuildConfigurationEvent();
       if (configuration == null) {
         configuration = new NullConfiguration();
       }
@@ -216,7 +216,7 @@ public BuildEventStreamProtos.BuildEvent asStreamProto(BuildEventContext convert
       actionBuilder.setLabel(action.getOwner().getLabel().toString());
     }
     if (action.getOwner() != null) {
-      BuildEvent configuration = action.getOwner().getConfiguration();
+      BuildEvent configuration = action.getOwner().getBuildConfigurationEvent();
       if (configuration == null) {
         configuration = new NullConfiguration();
       }

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

@@ -13,8 +13,6 @@
 // limitations under the License.
 package com.google.devtools.build.lib.actions;
 
-import static com.google.common.base.Preconditions.checkNotNull;
-
 import com.google.auto.value.AutoValue;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
@@ -34,82 +32,73 @@
  * but to avoid storing heavyweight analysis objects in actions, and to avoid coupling between the
  * analysis and actions packages, the RuleConfiguredTarget provides an instance of this class.
  */
-@AutoValue
+// TODO(b/274783642): Instead of storing `mnemonic`, `configurationChecksum`,
+//  `buildConfigurationEvent` and `additionalProgressInfo` fields, we can instead just store
+//  `BuildConfigurationValue` field in `ActionOwner` class, which saves 3 fields.
 @Immutable
 public abstract class ActionOwner {
   /** An action owner for special cases. Usage is strongly discouraged. */
   @SerializationConstant
   public static final ActionOwner SYSTEM_ACTION_OWNER =
-      ActionOwner.createInternal(
-          null,
-          ImmutableList.of(),
+      create(
+          /* label= */ null,
           Location.BUILTIN,
-          "system",
-          "empty target kind",
-          "system",
-          null,
-          null,
-          ImmutableMap.of(),
-          null);
+          /* targetKind= */ "empty target kind",
+          /* mnemonic= */ "system",
+          /* configurationChecksum= */ "system",
+          /* buildConfigurationEvent= */ null,
+          /* additionalProgressInfo= */ null,
+          /* executionPlatform= */ null,
+          /* aspectDescriptors= */ ImmutableList.of(),
+          /* execProperties= */ ImmutableMap.of());
 
   public static ActionOwner create(
-      Label label,
-      ImmutableList<AspectDescriptor> aspectDescriptors,
-      Location location,
-      String mnemonic,
-      String targetKind,
-      String configurationChecksum,
-      BuildConfigurationEvent configuration,
-      @Nullable String additionalProgressInfo,
-      ImmutableMap<String, String> execProperties,
-      @Nullable PlatformInfo executionPlatform) {
-    return createInternal(
-        checkNotNull(label),
-        aspectDescriptors,
-        checkNotNull(location),
-        checkNotNull(mnemonic),
-        checkNotNull(targetKind),
-        checkNotNull(configurationChecksum),
-        checkNotNull(configuration),
-        additionalProgressInfo,
-        execProperties,
-        executionPlatform);
-  }
-
-  private static ActionOwner createInternal(
       @Nullable Label label,
-      ImmutableList<AspectDescriptor> aspectDescriptors,
       Location location,
-      String mnemonic,
       String targetKind,
+      String mnemonic,
       String configurationChecksum,
-      @Nullable BuildConfigurationEvent configuration,
+      @Nullable BuildConfigurationEvent buildConfigurationEvent,
       @Nullable String additionalProgressInfo,
-      ImmutableMap<String, String> execProperties,
-      @Nullable PlatformInfo executionPlatform) {
-    return new AutoValue_ActionOwner(
-        location,
-        label,
-        aspectDescriptors,
-        mnemonic,
-        checkNotNull(configurationChecksum),
-        configuration,
-        targetKind,
-        additionalProgressInfo,
-        execProperties,
-        executionPlatform);
+      @Nullable PlatformInfo executionPlatform,
+      ImmutableList<AspectDescriptor> aspectDescriptors,
+      ImmutableMap<String, String> execProperties) {
+    if (aspectDescriptors.isEmpty() && execProperties.isEmpty()) {
+      return LiteActionOwner.createInternal(
+          label,
+          location,
+          targetKind,
+          mnemonic,
+          configurationChecksum,
+          buildConfigurationEvent,
+          additionalProgressInfo,
+          executionPlatform);
+    } else {
+      return FullActionOwner.createInternal(
+          label,
+          location,
+          targetKind,
+          mnemonic,
+          configurationChecksum,
+          buildConfigurationEvent,
+          additionalProgressInfo,
+          executionPlatform,
+          aspectDescriptors,
+          execProperties);
+    }
   }
 
-  /** Returns the location of this ActionOwner. */
-  public abstract Location getLocation();
-
   /** Returns the label for this ActionOwner, or null if the {@link #SYSTEM_ACTION_OWNER}. */
   @Nullable
   public abstract Label getLabel();
 
-  public abstract ImmutableList<AspectDescriptor> getAspectDescriptors();
+  /** Returns the location for this ActionOwner. */
+  public abstract Location getLocation();
 
-  /** Returns the configuration's mnemonic. */
+  /** Returns the target kind (rule class name) for this ActionOwner. */
+  public abstract String getTargetKind();
+
+  /** Returns the mnemonic for the configuration of the action owner. */
   public abstract String getMnemonic();
 
   /**
@@ -125,10 +114,7 @@ private static ActionOwner createInternal(
    * should be reported in the build event protocol.
    */
   @Nullable
-  public abstract BuildConfigurationEvent getConfiguration();
-
-  /** Returns the target kind (rule class name) for this ActionOwner. */
-  public abstract String getTargetKind();
+  public abstract BuildConfigurationEvent getBuildConfigurationEvent();
 
   /**
    * Returns additional information that should be displayed in progress messages, or {@code null}
@@ -137,14 +123,84 @@ private static ActionOwner createInternal(
   @Nullable
   abstract String getAdditionalProgressInfo();
 
-  /** Returns a String to String map containing the execution properties of this action. */
-  @VisibleForTesting
-  public abstract ImmutableMap<String, String> getExecProperties();
-
   /**
    * Returns the {@link PlatformInfo} platform this action should be executed on. If the execution
    * platform is {@code null}, then the host platform is assumed.
    */
   @Nullable
   public abstract PlatformInfo getExecutionPlatform();
+
+  public abstract ImmutableList<AspectDescriptor> getAspectDescriptors();
+
+  /** Returns a String to String map containing the execution properties of this action. */
+  @VisibleForTesting
+  public abstract ImmutableMap<String, String> getExecProperties();
+
+  /**
+   * Created when {@code aspectDescriptors} and {@code execProperties} are both empty.
+   *
+   * <p>{@link LiteActionOwner} is more likely to be created since both fields above are usually
+   * empty. This will save 8 bytes of memory for each {@link ActionOwner} instance compared to
+   * keeping both empty fields.
+   */
+  @AutoValue
+  abstract static class LiteActionOwner extends ActionOwner {
+    static LiteActionOwner createInternal(
+        @Nullable Label label,
+        Location location,
+        String targetKind,
+        String mnemonic,
+        String configurationChecksum,
+        @Nullable BuildConfigurationEvent buildConfigurationEvent,
+        @Nullable String additionalProgressInfo,
+        @Nullable PlatformInfo executionPlatform) {
+      return new AutoValue_ActionOwner_LiteActionOwner(
+          label,
+          location,
+          targetKind,
+          mnemonic,
+          configurationChecksum,
+          buildConfigurationEvent,
+          additionalProgressInfo,
+          executionPlatform);
+    }
+
+    @Override
+    public final ImmutableList<AspectDescriptor> getAspectDescriptors() {
+      return ImmutableList.of();
+    }
+
+    @Override
+    public final ImmutableMap<String, String> getExecProperties() {
+      return ImmutableMap.of();
+    }
+  }
+
+  /** Created when either {@code aspectDescriptors} or {@code execProperties} is not empty. */
+  @AutoValue
+  abstract static class FullActionOwner extends ActionOwner {
+    static FullActionOwner createInternal(
+        @Nullable Label label,
+        Location location,
+        String targetKind,
+        String mnemonic,
+        String configurationChecksum,
+        @Nullable BuildConfigurationEvent buildConfigurationEvent,
+        @Nullable String additionalProgressInfo,
+        @Nullable PlatformInfo executionPlatform,
+        ImmutableList<AspectDescriptor> aspectDescriptors,
+        ImmutableMap<String, String> execProperties) {
+      return new AutoValue_ActionOwner_FullActionOwner(
+          label,
+          location,
+          targetKind,
+          mnemonic,
+          configurationChecksum,
+          buildConfigurationEvent,
+          additionalProgressInfo,
+          executionPlatform,
+          aspectDescriptors,
+          execProperties);
+    }
+  }
 }

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

@@ -135,6 +135,7 @@ java_library(
         "//src/main/protobuf:extra_actions_base_java_proto",
         "//src/main/protobuf:failure_details_java_proto",
         "//third_party:auto_value",
+        "//third_party:checker_framework_annotations",
         "//third_party:flogger",
         "//third_party:guava",
         "//third_party:jsr305",

src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java

@@ -512,15 +512,15 @@ public static ActionOwner createActionOwner(
       @Nullable PlatformInfo executionPlatform) {
     return ActionOwner.create(
         rule.getLabel(),
-        aspectDescriptors,
         rule.getLocation(),
-        configuration.getMnemonic(),
         rule.getTargetKind(),
+        configuration.getMnemonic(),
         configuration.checksum(),
         configuration.toBuildEvent(),
         configuration.isToolConfiguration() ? TOOL_CONFIGURATION_PROGRESS_TAG : null,
-        execProperties,
-        executionPlatform);
+        executionPlatform,
+        aspectDescriptors,
+        execProperties);
   }
 
   @Override

src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphSummaryOutputFormatterCallback.java

@@ -93,7 +93,7 @@ private void processAction(ActionAnalysisMetadata action) throws InterruptedExce
     mnemonicToCount.merge(action.getMnemonic(), 1, Integer::sum);
     ActionOwner actionOwner = action.getOwner();
     if (actionOwner != null) {
-      BuildEvent configuration = actionOwner.getConfiguration();
+      BuildEvent configuration = actionOwner.getBuildConfigurationEvent();
       BuildEventStreamProtos.Configuration configProto =
           configuration.asStreamProto(/*context=*/ null).getConfiguration();
       configurationToCount.merge(configProto.getMnemonic(), 1, Integer::sum);

src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphTextOutputFormatterCallback.java

@@ -143,7 +143,7 @@ private void writeAction(ActionAnalysisMetadata action, PrintStream printStream)
         .append('\n');
 
     if (actionOwner != null) {
-      BuildEvent configuration = actionOwner.getConfiguration();
+      BuildEvent configuration = actionOwner.getBuildConfigurationEvent();
       BuildEventStreamProtos.Configuration configProto =
           configuration.asStreamProto(/*context=*/ null).getConfiguration();
 

src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/ActionGraphDump.java

@@ -182,7 +182,8 @@ private void dumpSingleAction(ConfiguredTarget configuredTarget, ActionAnalysisM
       // AbstractAction, we can call getEffectiveEnvironment here to handle these actions as well.
       // TODO(twerth): This handles the fixed environment. We probably want to output the inherited
       // environment as well.
-      ImmutableMap<String, String> fixedEnvironment = spawnAction.getEffectiveEnvironment(Map.of());
+      ImmutableMap<String, String> fixedEnvironment =
+          spawnAction.getEffectiveEnvironment(ImmutableMap.of());
       for (Map.Entry<String, String> environmentVariable : fixedEnvironment.entrySet()) {
         actionBuilder.addEnvironmentVariables(
             AnalysisProtosV2.KeyValuePair.newBuilder()
@@ -233,7 +234,7 @@ private void dumpSingleAction(ConfiguredTarget configuredTarget, ActionAnalysisM
 
     ActionOwner actionOwner = action.getOwner();
     if (actionOwner != null) {
-      BuildEvent event = actionOwner.getConfiguration();
+      BuildEvent event = actionOwner.getBuildConfigurationEvent();
       actionBuilder.setConfigurationId(knownConfigurations.dataToIdAndStreamOutputProto(event));
       if (actionOwner.getExecutionPlatform() != null) {
         actionBuilder.setExecutionPlatform(actionOwner.getExecutionPlatform().label().toString());

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

@@ -517,17 +517,17 @@ public String toString() {
   public static final ActionOwner NULL_ACTION_OWNER =
       ActionOwner.create(
           NULL_LABEL,
-          ImmutableList.of(),
           new Location("dummy-file", 0, 0),
-          "dummy-configuration-mnemonic",
-          "dummy-kind",
-          "dummy-configuration",
+          /* targetKind= */ "dummy-kind",
+          /* mnemonic= */ "dummy-configuration-mnemonic",
+          /* configurationChecksum= */ "dummy-configuration",
           new BuildConfigurationEvent(
               BuildEventStreamProtos.BuildEventId.getDefaultInstance(),
               BuildEventStreamProtos.BuildEvent.getDefaultInstance()),
-          null,
-          ImmutableMap.of(),
-          null);
+          /* additionalProgressInfo= */ null,
+          /* executionPlatform= */ null,
+          /* aspectDescriptors= */ ImmutableList.of(),
+          /* execProperties= */ ImmutableMap.of());
 
   @SerializationConstant
   public static final ActionLookupData NULL_ACTION_LOOKUP_DATA =

src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java

@@ -135,20 +135,20 @@ public void testExecutionInfo_fromExecutionPlatform() throws Exception {
     ActionOwner actionOwner =
         ActionOwner.create(
             Label.parseCanonicalUnchecked("//target"),
-            ImmutableList.of(),
             new Location("dummy-file", 0, 0),
-            "dummy-configuration-mnemonic",
-            "dummy-kind",
-            "dummy-configuration",
+            /* targetKind= */ "dummy-kind",
+            /* mnemonic= */ "dummy-configuration-mnemonic",
+            /* configurationChecksum= */ "dummy-configuration",
             new BuildConfigurationEvent(
                 BuildEventStreamProtos.BuildEventId.getDefaultInstance(),
                 BuildEventStreamProtos.BuildEvent.getDefaultInstance()),
-            null,
+            /* additionalProgressInfo= */ null,
+            /* executionPlatform= */ null,
+            ImmutableList.of(),
             ImmutableMap.<String, String>builder()
                 .put("prop1", "foo")
                 .put("prop2", "bar")
-                .buildOrThrow(),
-            null);
+                .buildOrThrow());
 
     SpawnAction action =
         builder()

src/test/java/com/google/devtools/build/lib/exec/util/FakeOwner.java

@@ -82,17 +82,17 @@ public FakeOwner(String mnemonic, String progressMessage, String ownerLabel) {
   public ActionOwner getOwner() {
     return ActionOwner.create(
         Label.parseCanonicalUnchecked(ownerLabel),
-        /* aspectDescriptors= */ ImmutableList.of(),
         new Location("dummy-file", 0, 0),
+        /* targetKind= */ "dummy-target-kind",
         mnemonic,
-        "dummy-target-kind",
-        "configurationChecksum",
+        /* configurationChecksum= */ "configurationChecksum",
         new BuildConfigurationEvent(
             BuildEventStreamProtos.BuildEventId.getDefaultInstance(),
             BuildEventStreamProtos.BuildEvent.getDefaultInstance()),
-        "additionalProgressInfo",
-        /* execProperties= */ ImmutableMap.of(),
-        null);
+        /* additionalProgressInfo= */ "additionalProgressInfo",
+        /* executionPlatform= */ null,
+        /* aspectDescriptors= */ ImmutableList.of(),
+        /* execProperties= */ ImmutableMap.of());
   }
 
   @Override