Skip to content

Commit

Permalink
Create precomputedBaselineExecConfiguration. This is a prerequisite f…
Browse files Browse the repository at this point in the history
…or preserving the analysis cache for exec-configured nodes when non-exec flags change.

PiperOrigin-RevId: 700795895
Change-Id: I1f2157149b1d20fb7aef71e2c29d9abd64c62190
  • Loading branch information
susinmotion authored and copybara-github committed Nov 27, 2024
1 parent 9eb994b commit 37d733f
Show file tree
Hide file tree
Showing 10 changed files with 91 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ public AnalysisResult update(
pollInterruptedStatus();

skyframeBuildView.resetProgressReceiver();
skyframeExecutor.setBaselineConfiguration(targetOptions);
skyframeExecutor.setBaselineConfiguration(targetOptions, eventHandler);

ImmutableMap.Builder<Label, Target> labelToTargetsMapBuilder =
ImmutableMap.builderWithExpectedSize(loadingResult.getTargetLabels().size());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ public BlazeCommandResult exec(
ensureSyncPackageLoading(env, optionsParsingResult);
// TODO(bazel-team): What if there are multiple configurations? [multi-config]
BuildOptions buildOptions = runtime.createBuildOptions(optionsParsingResult);
env.getSkyframeExecutor().setBaselineConfiguration(buildOptions);
env.getSkyframeExecutor().setBaselineConfiguration(buildOptions, env.getReporter());
return env.getSkyframeExecutor()
.getConfiguration(env.getReporter(), buildOptions, /* keepGoing= */ true);
} catch (InvalidConfigurationException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ public static <T> Injected injected(Precomputed<T> precomputed, T value) {
public static final Precomputed<BuildOptions> BASELINE_CONFIGURATION =
new Precomputed<>("baseline_configuration", /*shareable=*/ false);

// Unsharable because of complications in deserializing BuildOptions on startup due to caching.
public static final Precomputed<BuildOptions> BASELINE_EXEC_CONFIGURATION =
new Precomputed<>("baseline_exec_configuration", /* shareable= */ false);

private final Object value;

@VisibleForTesting
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1399,8 +1399,22 @@ private void setAutoloadsConfiguration(AutoloadSymbols autoloadSymbols) {
AutoloadSymbols.AUTOLOAD_SYMBOLS.set(injectable(), autoloadSymbols);
}

public void setBaselineConfiguration(BuildOptions buildOptions) {
public void setBaselineConfiguration(BuildOptions buildOptions, ExtendedEventHandler eventHandler)
throws InvalidConfigurationException, InterruptedException {
PrecomputedValue.BASELINE_CONFIGURATION.set(injectable(), buildOptions);
PrecomputedValue.BASELINE_EXEC_CONFIGURATION.set(
injectable(), adjustForExec(buildOptions, eventHandler));
}

public BuildOptions adjustForExec(BuildOptions buildOptions, ExtendedEventHandler eventHandler)
throws InvalidConfigurationException, InterruptedException {
StarlarkAttributeTransitionProvider execTransition;
try {
execTransition = getStarlarkExecTransition(buildOptions, eventHandler);
} catch (StarlarkExecTransitionLoadingException e) {
throw new InvalidConfigurationException(e);
}
return BaselineOptionsFunction.adjustForExec(buildOptions, execTransition, null, eventHandler);
}

public void injectExtraPrecomputedValues(List<PrecomputedValue.Injected> extraPrecomputedValues) {
Expand Down Expand Up @@ -2008,20 +2022,25 @@ public Collection<SkyKey> getTransitiveConfigurationKeys() {
* <p>Production code handles this in Bazel's analysis phase skyfunctions.
*/
@Nullable
@VisibleForTesting
public StarlarkAttributeTransitionProvider getStarlarkExecTransitionForTesting(
public StarlarkAttributeTransitionProvider getStarlarkExecTransition(
BuildOptions options, ExtendedEventHandler eventHandler)
throws StarlarkExecTransitionLoadingException, InterruptedException {
return StarlarkExecTransitionLoader.loadStarlarkExecTransition(
options,
(bzlKey) ->
(BzlLoadValue)
evaluate(
ImmutableList.of(bzlKey),
/* keepGoing= */ false,
/* numThreads= */ DEFAULT_THREAD_COUNT,
eventHandler)
.get(bzlKey))
(bzlKey) -> {
EvaluationResult<SkyValue> result =
evaluate(
ImmutableList.of(bzlKey),
/* keepGoing= */ false,
/* numThreads= */ DEFAULT_THREAD_COUNT,
eventHandler);
if (result.hasError()) {
if (result.getError(bzlKey).getException() instanceof BzlLoadFailedException e) {
throw e;
}
}
return (BzlLoadValue) result.get(bzlKey);
})
.orElse(null);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.google.devtools.build.lib.analysis.config.transitions.TransitionUtil;
import com.google.devtools.build.lib.analysis.starlark.StarlarkAttributeTransitionProvider;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.packages.AttributeTransitionData;
import com.google.devtools.build.lib.skyframe.BzlLoadFailedException;
import com.google.devtools.build.lib.skyframe.BzlLoadValue;
Expand Down Expand Up @@ -71,41 +72,28 @@ public SkyValue compute(SkyKey skyKey, Environment env)
if (mappedBaselineOptions == null) {
return null;
}

Optional<StarlarkAttributeTransitionProvider> starlarkExecTransition;
try {
starlarkExecTransition =
StarlarkExecTransitionLoader.loadStarlarkExecTransition(
mappedBaselineOptions,
(bzlKey) -> (BzlLoadValue) env.getValueOrThrow(bzlKey, BzlLoadFailedException.class));
} catch (StarlarkExecTransitionLoadingException e) {
throw new BaselineOptionsFunctionException(e);
}
if (starlarkExecTransition == null) {
return null;
}

// Next, apply elements of BaselineOptionsKey: apply exec transition and/or adjust platform
BuildOptions adjustedBaselineOptions = mappedBaselineOptions;

if (key.afterExecTransition()) {
// A null executionPlatform actually skips transition application so need some value here when
// not overriding the platform. It is safe to supply some fake value here (as long as it is
// constant) since the baseline should never be used to actually construct an action or do
// toolchain resolution.
PatchTransition execTransition =
ExecutionTransitionFactory.createFactory()
.create(
AttributeTransitionData.builder()
.executionPlatform(
key.newPlatform() != null
? key.newPlatform()
: Label.parseCanonicalUnchecked(
"//this_is_a_faked_exec_platform_for_blaze_internals"))
.analysisData(starlarkExecTransition.orElse(null))
.build());
Optional<StarlarkAttributeTransitionProvider> starlarkExecTransition;
try {
starlarkExecTransition =
StarlarkExecTransitionLoader.loadStarlarkExecTransition(
adjustedBaselineOptions,
(bzlKey) ->
(BzlLoadValue) env.getValueOrThrow(bzlKey, BzlLoadFailedException.class));
} catch (StarlarkExecTransitionLoadingException e) {
throw new BaselineOptionsFunctionException(e);
}
if (starlarkExecTransition == null) {
return null;
}
adjustedBaselineOptions =
execTransition.patch(
TransitionUtil.restrict(execTransition, adjustedBaselineOptions), env.getListener());
adjustForExec(
mappedBaselineOptions,
starlarkExecTransition.orElse(null),
key.newPlatform(),
env.getListener());
} else if (key.newPlatform() != null) {
// Clone for safety as-is the standard for all transitions.
adjustedBaselineOptions = adjustedBaselineOptions.clone();
Expand All @@ -131,6 +119,36 @@ public SkyValue compute(SkyKey skyKey, Environment env)
return BaselineOptionsValue.create(remappedAdjustedBaselineOptions);
}

/** Adjusts the baseline options for the exec transition. */
public static BuildOptions adjustForExec(
BuildOptions baselineOptions,
StarlarkAttributeTransitionProvider starlarkExecTransition,
Label newPlatform,
ExtendedEventHandler eventHandler)
throws InterruptedException {

// A null executionPlatform actually skips transition application so need some value here when
// not overriding the platform. It is safe to supply some fake value here (as long as it is
// constant) since the baseline should never be used to actually construct an action or do
// toolchain resolution.
PatchTransition execTransition =
ExecutionTransitionFactory.createFactory()
.create(
AttributeTransitionData.builder()
.executionPlatform(
newPlatform != null
? newPlatform
: Label.parseCanonicalUnchecked(
"//this_is_a_faked_exec_platform_for_blaze_internals"))
.analysisData(starlarkExecTransition)
.build());
baselineOptions =
execTransition.patch(
TransitionUtil.restrict(execTransition, baselineOptions), eventHandler);

return baselineOptions;
}

@Nullable
private static BuildOptions mapBuildOptions(Environment env, BuildOptions rawBaselineOptions)
throws InterruptedException, BaselineOptionsFunctionException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ private PatchTransition getExecTransition(Label execPlatform) throws Exception {
.attributes(FakeAttributeMapper.empty())
.analysisData(
getSkyframeExecutor()
.getStarlarkExecTransitionForTesting(targetConfig.getOptions(), reporter))
.getStarlarkExecTransition(targetConfig.getOptions(), reporter))
.executionPlatform(execPlatform)
.build());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,7 @@ public void composeCommutativelyWithExecutionTransition() throws Exception {
.attributes(FakeAttributeMapper.empty())
.analysisData(
getSkyframeExecutor()
.getStarlarkExecTransitionForTesting(
targetConfig.getOptions(), reporter))
.getStarlarkExecTransition(targetConfig.getOptions(), reporter))
.executionPlatform(executionPlatform)
.build());
assertThat(execTransition).isNotNull();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -545,8 +545,7 @@ public static BuildOptions execOptions(
.attributes(FakeAttributeMapper.empty())
.executionPlatform(targetOptions.get(PlatformOptions.class).hostPlatform)
.analysisData(
skyframeExecutor.getStarlarkExecTransitionForTesting(
targetOptions, handler))
skyframeExecutor.getStarlarkExecTransition(targetOptions, handler))
.build())
.apply(new BuildOptionsView(targetOptions, targetOptions.getFragmentClasses()), handler)
.values());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ protected final BuildConfigurationValue createConfiguration(String... args) thro

// This is being done outside of BuildView, potentially even before the BuildView was
// constructed and thus cannot rely on BuildView having injected this for us.
skyframeExecutor.setBaselineConfiguration(buildOptions);
skyframeExecutor.setBaselineConfiguration(buildOptions, reporter);
return skyframeExecutor.createConfiguration(reporter, buildOptions, false);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ protected BuildConfigurationValue createConfiguration(
BuildOptions targetOptions = parseBuildOptions(starlarkOptions, args);

skyframeExecutor.handleDiffsForTesting(reporter);
skyframeExecutor.setBaselineConfiguration(targetOptions);
skyframeExecutor.setBaselineConfiguration(targetOptions, reporter);
return skyframeExecutor.createConfiguration(reporter, targetOptions, false);
}

Expand Down

0 comments on commit 37d733f

Please sign in to comment.