diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AspectValue.java b/src/main/java/com/google/devtools/build/lib/analysis/AspectValue.java index a7dea427be97bc..2999ebf5e2412a 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/AspectValue.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/AspectValue.java @@ -38,6 +38,17 @@ public static AspectValue create( : new AspectValueWithTransitivePackages(key, aspect, configuredAspect, transitivePackages); } + public static AspectValue createForAlias( + AspectKey key, + Aspect aspect, + ConfiguredAspect configuredAspect, + @Nullable NestedSet transitivePackages) { + return transitivePackages == null + ? new AspectValueForAlias(aspect, configuredAspect) + : new AspectValueWithTransitivePackagesForAlias( + key, aspect, configuredAspect, transitivePackages); + } + // These variables are only non-final because they may be clear()ed to save memory. They are null // only after they are cleared except for transitivePackagesForPackageRootResolution. @Nullable private Aspect aspect; @@ -101,7 +112,7 @@ public String toString() { return getStringHelper().toString(); } - private static final class AspectValueWithTransitivePackages extends AspectValue { + private static class AspectValueWithTransitivePackages extends AspectValue { @Nullable private transient NestedSet transitivePackages; // Null after clear(). @Nullable private AspectKey key; @@ -137,4 +148,26 @@ protected ToStringHelper getStringHelper() { return super.getStringHelper().add("key", key).add("transitivePackages", transitivePackages); } } + + private static final class AspectValueForAlias extends AspectValue { + private AspectValueForAlias(Aspect aspect, ConfiguredAspect configuredAspect) { + super(aspect, configuredAspect); + } + } + + private static final class AspectValueWithTransitivePackagesForAlias + extends AspectValueWithTransitivePackages { + private AspectValueWithTransitivePackagesForAlias( + AspectKey key, + Aspect aspect, + ConfiguredAspect configuredAspect, + NestedSet transitivePackages) { + super(key, aspect, configuredAspect, transitivePackages); + } + } + + public static boolean isForAliasTarget(AspectValue aspectValue) { + return aspectValue instanceof AspectValueForAlias + || aspectValue instanceof AspectValueWithTransitivePackagesForAlias; + } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java index 4ff3a0fa385fd1..f0b5bab4bbb313 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java @@ -927,7 +927,7 @@ private AspectValue createAliasAspect( .addTransitive(real.getTransitivePackages()) .build() : null; - return AspectValue.create( + return AspectValue.createForAlias( originalKey, aspect, ConfiguredAspect.forAlias(real), transitivePackages); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java index 28b3741747d2cd..3c9ce5a25ff25f 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java @@ -49,6 +49,7 @@ import com.google.devtools.build.lib.analysis.AnalysisFailureEvent; import com.google.devtools.build.lib.analysis.AnalysisOperationWatcher; import com.google.devtools.build.lib.analysis.AnalysisPhaseCompleteEvent; +import com.google.devtools.build.lib.analysis.AspectValue; import com.google.devtools.build.lib.analysis.CachingAnalysisEnvironment; import com.google.devtools.build.lib.analysis.ConfiguredAspect; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; @@ -1506,6 +1507,12 @@ public void evaluated( } configuredObjectCount.incrementAndGet(); if (newValue instanceof ActionLookupValue alv) { + if (alv instanceof AspectValue) { + if (AspectValue.isForAliasTarget((AspectValue) alv)) { + // Created actions will be counted from {@link AspectValue} on the original target. + return; + } + } // During multithreaded operation, this is only set to true, so no concurrency issues. someActionLookupValueEvaluated = true; ImmutableList actions = alv.getActions(); diff --git a/src/test/java/com/google/devtools/build/lib/metrics/MetricsCollectorTest.java b/src/test/java/com/google/devtools/build/lib/metrics/MetricsCollectorTest.java index de4bb1943d232b..c8694cc9938e49 100644 --- a/src/test/java/com/google/devtools/build/lib/metrics/MetricsCollectorTest.java +++ b/src/test/java/com/google/devtools/build/lib/metrics/MetricsCollectorTest.java @@ -180,6 +180,54 @@ def _impl(ctx): .isEqualTo(buildMetrics.getActionSummary().getActionsExecuted()); } + @Test + public void testAspectActionsCreatedNotOverCountedForAlias() throws Exception { + write( + "pkg/BUILD", + """ + load(":defs.bzl", "my_rule") + my_rule(name = "foo", dep = "dep_alias_alias") + alias(name = "dep_alias_alias", actual = ":dep_alias") + alias(name = "dep_alias", actual = ":dep") + my_rule(name = "dep") + """); + write( + "pkg/defs.bzl", + """ + def _aspect_impl(target, ctx): + f = ctx.actions.declare_file(target.label.name + ".out") + ctx.actions.run_shell( + outputs = [f], + command = "touch $@", + mnemonic = "MyAspectAction", + ) + return [OutputGroupInfo(my_out = depset([f]))] + + my_aspect = aspect(implementation = _aspect_impl) + + def _impl(ctx): + pass + + my_rule = rule( + implementation = _impl, + attrs = { + "dep": attr.label(aspects = [my_aspect]), + }, + ) + """); + buildTarget("//pkg:foo"); + + BuildMetrics buildMetrics = buildMetricsEventListener.event.getBuildMetrics(); + List actionData = buildMetrics.getActionSummary().getActionDataList(); + ImmutableList aspectActions = + actionData.stream() + .filter(a -> a.getMnemonic().equals("MyAspectAction")) + .collect(toImmutableList()); + assertThat(aspectActions).hasSize(1); + ActionData aspectAction = aspectActions.get(0); + assertThat(aspectAction.getActionsCreated()).isEqualTo(1); + } + @Test public void buildGraphAndArtifactMetrics() throws Exception { write(