From a0a65944b92325e6be636055156450a3059ffab6 Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 20 Apr 2023 11:01:35 -0700 Subject: [PATCH] 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 --- .../lib/actions/ActionExecutedEvent.java | 4 +- .../build/lib/actions/ActionOwner.java | 188 ++++++++++++------ .../google/devtools/build/lib/actions/BUILD | 1 + .../build/lib/analysis/RuleContext.java | 8 +- ...onGraphSummaryOutputFormatterCallback.java | 2 +- ...ctionGraphTextOutputFormatterCallback.java | 2 +- .../actiongraph/v2/ActionGraphDump.java | 5 +- .../lib/actions/util/ActionsTestUtil.java | 14 +- .../lib/analysis/actions/SpawnActionTest.java | 14 +- .../build/lib/exec/util/FakeOwner.java | 12 +- .../build/lib/runtime/UiStateTrackerTest.java | 70 +++---- .../build/lib/testutil/FakeResourceOwner.java | 18 +- .../devtools/build/lib/worker/TestUtils.java | 20 -- 13 files changed, 198 insertions(+), 160 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutedEvent.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutedEvent.java index 3ff7f01ad02de4..f155fcc7ea78b2 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutedEvent.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutedEvent.java @@ -137,7 +137,7 @@ public Collection getChildrenEvents() { @Override public Collection 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(); } diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java b/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java index 9623ef166eefb1..500ad0cda285f3 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java +++ b/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 aspectDescriptors, - Location location, - String mnemonic, - String targetKind, - String configurationChecksum, - BuildConfigurationEvent configuration, - @Nullable String additionalProgressInfo, - ImmutableMap 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 aspectDescriptors, Location location, - String mnemonic, String targetKind, + String mnemonic, String configurationChecksum, - @Nullable BuildConfigurationEvent configuration, + @Nullable BuildConfigurationEvent buildConfigurationEvent, @Nullable String additionalProgressInfo, - ImmutableMap execProperties, - @Nullable PlatformInfo executionPlatform) { - return new AutoValue_ActionOwner( - location, - label, - aspectDescriptors, - mnemonic, - checkNotNull(configurationChecksum), - configuration, - targetKind, - additionalProgressInfo, - execProperties, - executionPlatform); + @Nullable PlatformInfo executionPlatform, + ImmutableList aspectDescriptors, + ImmutableMap 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 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 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 getAspectDescriptors(); + + /** Returns a String to String map containing the execution properties of this action. */ + @VisibleForTesting + public abstract ImmutableMap getExecProperties(); + + /** + * Created when {@code aspectDescriptors} and {@code execProperties} are both empty. + * + *

{@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 getAspectDescriptors() { + return ImmutableList.of(); + } + + @Override + public final ImmutableMap 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 aspectDescriptors, + ImmutableMap execProperties) { + return new AutoValue_ActionOwner_FullActionOwner( + label, + location, + targetKind, + mnemonic, + configurationChecksum, + buildConfigurationEvent, + additionalProgressInfo, + executionPlatform, + aspectDescriptors, + execProperties); + } + } } diff --git a/src/main/java/com/google/devtools/build/lib/actions/BUILD b/src/main/java/com/google/devtools/build/lib/actions/BUILD index f2df5192668edb..abd37cc320eb3c 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/BUILD +++ b/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", diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java index 5d504d8223e992..4aa12dc9a2bf37 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java +++ b/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 diff --git a/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphSummaryOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphSummaryOutputFormatterCallback.java index b1e2f62b531b57..4052216f54bc39 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphSummaryOutputFormatterCallback.java +++ b/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); diff --git a/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphTextOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphTextOutputFormatterCallback.java index 24b67f369ba9d7..6cc9d497c2f72a 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphTextOutputFormatterCallback.java +++ b/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(); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/ActionGraphDump.java b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/ActionGraphDump.java index 189846192157ed..fa1fbd40b98f42 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/ActionGraphDump.java +++ b/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 fixedEnvironment = spawnAction.getEffectiveEnvironment(Map.of()); + ImmutableMap fixedEnvironment = + spawnAction.getEffectiveEnvironment(ImmutableMap.of()); for (Map.Entry 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()); 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 59ff9e43db818d..a38c1a99069f92 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 @@ -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 = diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java index b0b3c6e941ea0d..edfd1d7b8cd5c0 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java +++ b/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.builder() .put("prop1", "foo") .put("prop2", "bar") - .buildOrThrow(), - null); + .buildOrThrow()); SpawnAction action = builder() diff --git a/src/test/java/com/google/devtools/build/lib/exec/util/FakeOwner.java b/src/test/java/com/google/devtools/build/lib/exec/util/FakeOwner.java index 5c40f000e03a9b..b39c1a97fe420b 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/util/FakeOwner.java +++ b/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 diff --git a/src/test/java/com/google/devtools/build/lib/runtime/UiStateTrackerTest.java b/src/test/java/com/google/devtools/build/lib/runtime/UiStateTrackerTest.java index b747eeddfc7187..d38af609826b97 100644 --- a/src/test/java/com/google/devtools/build/lib/runtime/UiStateTrackerTest.java +++ b/src/test/java/com/google/devtools/build/lib/runtime/UiStateTrackerTest.java @@ -205,17 +205,17 @@ private Action mockAction(String progressMessage, String primaryOutput) { private ActionOwner dummyActionOwner() throws LabelSyntaxException { return ActionOwner.create( Label.parseCanonical("//foo:a"), - ImmutableList.of(), new Location("dummy-file", 0, 0), - /*mnemonic=*/ "", - /*targetKind=*/ "", - /*configurationChecksum=*/ "", + /* targetKind= */ "", + /* mnemonic= */ "", + /* configurationChecksum= */ "", new BuildConfigurationEvent( BuildEventStreamProtos.BuildEventId.getDefaultInstance(), BuildEventStreamProtos.BuildEvent.getDefaultInstance()), - /*additionalProgressInfo=*/ "", - ImmutableMap.of(), - /*executionPlatform=*/ null); + /* additionalProgressInfo= */ "", + /* executionPlatform= */ null, + /* aspectDescriptors= */ ImmutableList.of(), + /* execProperties= */ ImmutableMap.of()); } private void simulateExecutionPhase(UiStateTracker uiStateTracker) { @@ -638,17 +638,17 @@ public void testSensibleShortening() throws Exception { ActionOwner owner = ActionOwner.create( label, - ImmutableList.of(), new Location("dummy-file", 0, 0), - "dummy-mnemonic", - "dummy-target-kind", - "fedcba", + /* targetKind= */ "dummy-target-kind", + /* mnemonic= */ "dummy-mnemonic", + /* configurationChecksum= */ "fedcba", new BuildConfigurationEvent( BuildEventStreamProtos.BuildEventId.getDefaultInstance(), BuildEventStreamProtos.BuildEvent.getDefaultInstance()), - null, - ImmutableMap.of(), - null); + /* additionalProgressInfo= */ null, + /* executionPlatform= */ null, + /* aspectDescriptors= */ ImmutableList.of(), + /* execProperties= */ ImmutableMap.of()); when(action.getOwner()).thenReturn(owner); clock.advanceMillis(TimeUnit.SECONDS.toMillis(3)); @@ -1184,17 +1184,17 @@ public void testAggregation() throws Exception { ActionOwner fooOwner = ActionOwner.create( labelFooTest, - ImmutableList.of(), new Location("dummy-file", 0, 0), - "TestRunner", - "dummy-target-kind", - "abcdef", + /* targetKind= */ "dummy-target-kind", + /* mnemonic= */ "TestRunner", + /* configurationChecksum= */ "abcdef", new BuildConfigurationEvent( BuildEventStreamProtos.BuildEventId.getDefaultInstance(), BuildEventStreamProtos.BuildEvent.getDefaultInstance()), - null, - ImmutableMap.of(), - null); + /* additionalProgressInfo= */ null, + /* executionPlatform= */ null, + /* aspectDescriptors= */ ImmutableList.of(), + /* execProperties= */ ImmutableMap.of()); Label labelBarTest = Label.parseCanonical("//baz:bartest"); ConfiguredTarget targetBarTest = mock(ConfiguredTarget.class); @@ -1202,17 +1202,17 @@ public void testAggregation() throws Exception { ActionOwner barOwner = ActionOwner.create( labelBarTest, - ImmutableList.of(), new Location("dummy-file", 0, 0), - "TestRunner", - "dummy-target-kind", - "fedcba", + /* targetKind= */ "dummy-target-kind", + /* mnemonic= */ "TestRunner", + /* configurationChecksum= */ "abcdef", new BuildConfigurationEvent( BuildEventStreamProtos.BuildEventId.getDefaultInstance(), BuildEventStreamProtos.BuildEvent.getDefaultInstance()), - null, - ImmutableMap.of(), - null); + /* additionalProgressInfo= */ null, + /* executionPlatform= */ null, + /* aspectDescriptors= */ ImmutableList.of(), + /* execProperties= */ ImmutableMap.of()); Label labelBazTest = Label.parseCanonical("//baz:baztest"); ConfiguredTarget targetBazTest = mock(ConfiguredTarget.class); @@ -1220,17 +1220,17 @@ public void testAggregation() throws Exception { ActionOwner bazOwner = ActionOwner.create( labelBazTest, - ImmutableList.of(), new Location("dummy-file", 0, 0), - "NonTestAction", - "dummy-target-kind", - "fedcba", + /* targetKind= */ "dummy-target-kind", + /* mnemonic= */ "NonTestAction", + /* configurationChecksum= */ "fedcba", new BuildConfigurationEvent( BuildEventStreamProtos.BuildEventId.getDefaultInstance(), BuildEventStreamProtos.BuildEvent.getDefaultInstance()), - null, - ImmutableMap.of(), - null); + /* additionalProgressInfo= */ null, + /* executionPlatform= */ null, + /* aspectDescriptors= */ ImmutableList.of(), + /* execProperties= */ ImmutableMap.of()); TestFilteringCompleteEvent filteringComplete = mock(TestFilteringCompleteEvent.class); when(filteringComplete.getTestTargets()) diff --git a/src/test/java/com/google/devtools/build/lib/testutil/FakeResourceOwner.java b/src/test/java/com/google/devtools/build/lib/testutil/FakeResourceOwner.java index 876d201c3b0bcb..1283d0b4267137 100644 --- a/src/test/java/com/google/devtools/build/lib/testutil/FakeResourceOwner.java +++ b/src/test/java/com/google/devtools/build/lib/testutil/FakeResourceOwner.java @@ -76,16 +76,16 @@ public boolean discoversInputs() { @Override public ActionOwner getOwner() { return ActionOwner.create( - null, - ImmutableList.of(), + /* label= */ null, Location.BUILTIN, - "fake", - "fake target kind", - "fake", - null, - null, - ImmutableMap.of(), - null); + /* targetKind= */ "fake target kind", + /* mnemonic= */ "fake", + /* configurationChecksum= */ "fake", + /* buildConfigurationEvent= */ null, + /* additionalProgressInfo= */ null, + /* executionPlatform= */ null, + /* aspectDescriptors= */ ImmutableList.of(), + /* execProperties= */ ImmutableMap.of()); } @Override diff --git a/src/test/java/com/google/devtools/build/lib/worker/TestUtils.java b/src/test/java/com/google/devtools/build/lib/worker/TestUtils.java index 752f03b57c878b..b717b2bd0e880f 100644 --- a/src/test/java/com/google/devtools/build/lib/worker/TestUtils.java +++ b/src/test/java/com/google/devtools/build/lib/worker/TestUtils.java @@ -19,16 +19,12 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; import com.google.common.hash.HashCode; -import com.google.devtools.build.lib.actions.ActionOwner; -import com.google.devtools.build.lib.actions.BuildConfigurationEvent; import com.google.devtools.build.lib.actions.ExecutionRequirements; import com.google.devtools.build.lib.actions.ExecutionRequirements.WorkerProtocolFormat; import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.actions.SimpleSpawn; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; -import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos; -import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.shell.Subprocess; @@ -40,7 +36,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import net.starlark.java.syntax.Location; /** Utilities that come in handy when unit-testing the worker code. */ class TestUtils { @@ -64,21 +59,6 @@ static Spawn createSpawn( ResourceSet.ZERO); } - static ActionOwner createActionOwner(String mnemonic) { - return ActionOwner.create( - Label.parseCanonicalUnchecked("//null/action:owner"), - ImmutableList.of(), - new Location("dummy-file", 0, 0), - mnemonic, - "dummy-kind", - "dummy-configuration", - new BuildConfigurationEvent( - BuildEventStreamProtos.BuildEventId.getDefaultInstance(), - BuildEventStreamProtos.BuildEvent.getDefaultInstance()), - null, - ImmutableMap.of(), - null); - } /** A helper method to create a WorkerKey through WorkerParser. */ static WorkerKey createWorkerKey( WorkerProtocolFormat protocolFormat,