diff --git a/src/main/java/com/google/devtools/build/lib/analysis/producers/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/producers/BUILD index 3360fa203caafa..375c3943365fba 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/producers/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/producers/BUILD @@ -115,6 +115,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/skyframe:build_options_scope_function", "//src/main/java/com/google/devtools/build/lib/skyframe:build_options_scope_value", + "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value", "//src/main/java/com/google/devtools/build/lib/skyframe/config", "//src/main/java/com/google/devtools/build/lib/skyframe/config:exceptions", "//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:platform_lookup_util", diff --git a/src/main/java/com/google/devtools/build/lib/analysis/producers/BuildConfigurationKeyProducer.java b/src/main/java/com/google/devtools/build/lib/analysis/producers/BuildConfigurationKeyProducer.java index 0f1deb65696386..b21b8ccc07ccce 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/producers/BuildConfigurationKeyProducer.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/producers/BuildConfigurationKeyProducer.java @@ -22,6 +22,7 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.skyframe.BuildOptionsScopeFunction.BuildOptionsScopeFunctionException; import com.google.devtools.build.lib.skyframe.BuildOptionsScopeValue; +import com.google.devtools.build.lib.skyframe.PrecomputedValue; import com.google.devtools.build.lib.skyframe.config.BuildConfigurationKey; import com.google.devtools.build.lib.skyframe.config.ParsedFlagsValue; import com.google.devtools.build.lib.skyframe.config.PlatformMappingException; @@ -105,6 +106,7 @@ public interface ResultSink { private PlatformMappingValue platformMappingValue; private BuildOptionsScopeValue buildOptionsScopeValue; private BuildOptions postPlatformProcessedOptions; + private BuildOptions baselineConfiguration; BuildConfigurationKeyProducer( ResultSink sink, StateMachine runAfter, C context, BuildOptions options, Label label) { @@ -164,7 +166,7 @@ private StateMachine findBuildOptionsScopes(Tasks tasks) { Preconditions.checkNotNull(this.postPlatformProcessedOptions); // including platform-based flags in skykey for scopes lookUp if (postPlatformProcessedOptions.getStarlarkOptions().isEmpty()) { - return this::finishConfigurationKeyProcessing; + return this::possiblyApplyScopes; } // the list of flags that are either project scoped or their scopes are not yet resolved. @@ -184,14 +186,14 @@ private StateMachine findBuildOptionsScopes(Tasks tasks) { // if flagsWithIncompleteScopeInfo is empty, we do not need to do any further lookUp for the // ScopeType and ScopeDefinition if (flagsWithIncompleteScopeInfo.isEmpty()) { - return this::finishConfigurationKeyProcessing; + return this::possiblyApplyScopes; } BuildOptionsScopeValue.Key buildOptionsScopeValueKey = BuildOptionsScopeValue.Key.create( this.postPlatformProcessedOptions, flagsWithIncompleteScopeInfo); tasks.lookUp(buildOptionsScopeValueKey, (Consumer) this); - return this::finishConfigurationKeyProcessing; + return this::possiblyApplyScopes; } /** @@ -258,28 +260,12 @@ public void accept(SkyValue value) { this.buildOptionsScopeValue = (BuildOptionsScopeValue) value; } - private StateMachine finishConfigurationKeyProcessing(Tasks tasks) { - if (this.postPlatformProcessedOptions.getStarlarkOptions().isEmpty()) { - sink.acceptTransitionedConfiguration( - this.context, BuildConfigurationKey.create(this.postPlatformProcessedOptions)); - return this.runAfter; - } - - BuildConfigurationKey finalBuildConfigurationKey = - possiblyApplyScopes( - this.buildOptionsScopeValue, this.label, this.postPlatformProcessedOptions); - sink.acceptTransitionedConfiguration(this.context, finalBuildConfigurationKey); - return this.runAfter; - } - - private BuildConfigurationKey possiblyApplyScopes( - @Nullable BuildOptionsScopeValue buildOptionsScopeValue, - Label label, - BuildOptions postPlatformBasedFlagsOptions) { + private StateMachine possiblyApplyScopes(Tasks tasks) { // This is not the same as null associated with Skyframe lookUp. This happens when scoping logic // is not enabled. This means the lookup via BuildOptionsScopesFunction was not performed. - if (buildOptionsScopeValue == null) { - return BuildConfigurationKey.create(postPlatformBasedFlagsOptions); + if (buildOptionsScopeValue == null + || postPlatformProcessedOptions.getStarlarkOptions().isEmpty()) { + return finishConfigurationKeyProcessing(postPlatformProcessedOptions); } boolean shouldApplyScopes = @@ -287,20 +273,29 @@ private BuildConfigurationKey possiblyApplyScopes( .anyMatch(scope -> scope.getScopeType() == Scope.ScopeType.PROJECT); if (!shouldApplyScopes) { - return BuildConfigurationKey.create( - this.buildOptionsScopeValue.getResolvedBuildOptionsWithScopeTypes()); + return finishConfigurationKeyProcessing( + buildOptionsScopeValue.getResolvedBuildOptionsWithScopeTypes()); } - if (!buildOptionsScopeValue - .getBaselineConfiguration() - .getStarlarkOptions() - .equals( - buildOptionsScopeValue.getResolvedBuildOptionsWithScopeTypes().getStarlarkOptions())) { - return BuildConfigurationKey.create(resetFlags(buildOptionsScopeValue, label)); - } + // TODO: b/390669368 - The same performance issue still exists if we reach this point. + tasks.lookUp( + PrecomputedValue.BASELINE_CONFIGURATION.getKey(), + val -> this.baselineConfiguration = (BuildOptions) ((PrecomputedValue) val).get()); + return this::applyScopes; + } + + private StateMachine applyScopes(Tasks tasks) { + BuildOptions resolved = buildOptionsScopeValue.getResolvedBuildOptionsWithScopeTypes(); + BuildOptions finalBuildOptions = + baselineConfiguration.getStarlarkOptions().equals(resolved.getStarlarkOptions()) + ? resolved + : resetFlags(buildOptionsScopeValue, baselineConfiguration, label); + return finishConfigurationKeyProcessing(finalBuildOptions); + } - return BuildConfigurationKey.create( - buildOptionsScopeValue.getResolvedBuildOptionsWithScopeTypes()); + private StateMachine finishConfigurationKeyProcessing(BuildOptions finalBuildOptions) { + sink.acceptTransitionedConfiguration(context, BuildConfigurationKey.create(finalBuildOptions)); + return runAfter; } /** @@ -318,7 +313,9 @@ private BuildConfigurationKey possiblyApplyScopes( * has the {@link Scope.ScopeType} information for all starlark flags. */ private static BuildOptions resetFlags( - BuildOptionsScopeValue buildOptionsScopeValue, Label label) { + BuildOptionsScopeValue buildOptionsScopeValue, + BuildOptions baselineConfiguration, + Label label) { Preconditions.checkNotNull(buildOptionsScopeValue); Preconditions.checkNotNull(label); @@ -329,7 +326,6 @@ private static BuildOptions resetFlags( return transitionedOptionsWithScopeType; } - BuildOptions baselineConfiguration = buildOptionsScopeValue.getBaselineConfiguration(); Preconditions.checkNotNull(baselineConfiguration); boolean flagsRemoved = false; boolean flagsResetToBaseline = false; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildOptionsScopeFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildOptionsScopeFunction.java index 1f1b7ef1809956..d1a1ecdbb0864d 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BuildOptionsScopeFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BuildOptionsScopeFunction.java @@ -120,11 +120,8 @@ public SkyValue compute(SkyKey skyKey, Environment env) : new Scope.ScopeDefinition(projectValue.getDefaultActiveDirectory()))); } - BuildOptions baseline = PrecomputedValue.BASELINE_CONFIGURATION.get(env); - return BuildOptionsScopeValue.create( fullyResolvedBuildOptionsBuilder.build(), - baseline, Lists.newArrayList(projectValueSkyKeysMap.keySet()), scopes); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildOptionsScopeValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildOptionsScopeValue.java index adc613fbc0352d..bc4e44dffc5f76 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BuildOptionsScopeValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BuildOptionsScopeValue.java @@ -20,7 +20,6 @@ import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.skyframe.SkyFunctionName; import com.google.devtools.build.skyframe.SkyKey; -import com.google.devtools.build.skyframe.SkyKey.SkyKeyInterner; import com.google.devtools.build.skyframe.SkyValue; import java.util.LinkedHashMap; import java.util.List; @@ -30,7 +29,6 @@ public final class BuildOptionsScopeValue implements SkyValue { BuildOptions resolvedBuildOptionsWithScopeTypes; - BuildOptions baselineConfiguration; List