From d6254dc384243722b0a5360ffdcddab41b5c21df Mon Sep 17 00:00:00 2001 From: Googler Date: Fri, 29 Sep 2023 13:37:55 -0700 Subject: [PATCH] Update PlatformInfoProducer to not use a configuration key. All platforms are non-configured (since https://github.com/bazelbuild/bazel/commit/87fb4620c6edd0e525d4621ecdca4bd47d69484c), so we can use an empty configuration and de-duplicate requests. Work towards platform-based flags: #19409. PiperOrigin-RevId: 569577077 Change-Id: Iea70d2c1484df17bc12925317415fc32cbd90ccc --- .../build/lib/analysis/producers/BUILD | 1 + ...ContextProducerWithCompatibilityCheck.java | 5 +-- .../producers/PlatformInfoProducer.java | 31 +++++++++++++------ .../TargetAndConfigurationProducer.java | 8 +---- 4 files changed, 25 insertions(+), 20 deletions(-) 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 98cb507fb3653b..cacc40b56960b7 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 @@ -24,6 +24,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis:config/config_conditions", "//src/main/java/com/google/devtools/build/lib/analysis:config/config_matching_provider", "//src/main/java/com/google/devtools/build/lib/analysis:config/configuration_transition_event", + "//src/main/java/com/google/devtools/build/lib/analysis:config/common_options", "//src/main/java/com/google/devtools/build/lib/analysis:config/core_options", "//src/main/java/com/google/devtools/build/lib/analysis:config/invalid_configuration_exception", "//src/main/java/com/google/devtools/build/lib/analysis:config/starlark_transition_cache", diff --git a/src/main/java/com/google/devtools/build/lib/analysis/producers/DependencyContextProducerWithCompatibilityCheck.java b/src/main/java/com/google/devtools/build/lib/analysis/producers/DependencyContextProducerWithCompatibilityCheck.java index 581e617007e738..e88bb64642fbf2 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/producers/DependencyContextProducerWithCompatibilityCheck.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/producers/DependencyContextProducerWithCompatibilityCheck.java @@ -96,10 +96,7 @@ public StateMachine step(Tasks tasks) { // Checks for incompatibility before toolchain resolution so that known missing // toolchains mark the target incompatible instead of failing the build. return new PlatformInfoProducer( - ConfiguredTargetKey.builder() - .setLabel(platformConfiguration.getTargetPlatform()) - .setConfigurationKey(defaultToolchainContextKey.configurationKey()) - .build(), + platformConfiguration.getTargetPlatform(), (PlatformInfoProducer.ResultSink) this, /* runAfter= */ this::computeConfigConditions); } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/producers/PlatformInfoProducer.java b/src/main/java/com/google/devtools/build/lib/analysis/producers/PlatformInfoProducer.java index 443dfc2f217a0f..5bd746011d7d1a 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/producers/PlatformInfoProducer.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/producers/PlatformInfoProducer.java @@ -15,10 +15,13 @@ import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.ConfiguredTargetValue; +import com.google.devtools.build.lib.analysis.config.CommonOptions; import com.google.devtools.build.lib.analysis.platform.PlatformInfo; import com.google.devtools.build.lib.analysis.platform.PlatformProviderUtils; +import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.packages.NoSuchTargetException; +import com.google.devtools.build.lib.skyframe.BuildConfigurationKey; import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey; import com.google.devtools.build.lib.skyframe.ConfiguredValueCreationException; import com.google.devtools.build.lib.skyframe.PackageValue; @@ -29,7 +32,10 @@ import javax.annotation.Nullable; /** - * Retrieves {@link PlatformInfo} for a given platform key. + * Retrieves {@link PlatformInfo} for a given platform. + * + *

Since platforms do not rely on the configuration, this uses a dummy blank configuration to + * help reduce the number of skyframe edges created. * *

This creates an explicit dependency on the {@link Package} to retrieve the associated target, * so it is possible to verify that {@link PlatformInfo} is an advertised provider before @@ -44,7 +50,7 @@ interface ResultSink { } // -------------------- Input -------------------- - private final ConfiguredTargetKey platformKey; + private final Label platformLabel; // -------------------- Output -------------------- private final ResultSink sink; @@ -56,8 +62,8 @@ interface ResultSink { private boolean passedValidation = false; private ConfiguredTarget platform; - PlatformInfoProducer(ConfiguredTargetKey platformKey, ResultSink sink, StateMachine runAfter) { - this.platformKey = platformKey; + PlatformInfoProducer(Label platformLabel, ResultSink sink, StateMachine runAfter) { + this.platformLabel = platformLabel; this.sink = sink; this.runAfter = runAfter; } @@ -69,7 +75,7 @@ public StateMachine step(Tasks tasks) { // // In distributed analysis, these packages will be duplicated across shards. tasks.lookUp( - platformKey.getLabel().getPackageIdentifier(), + platformLabel.getPackageIdentifier(), NoSuchPackageException.class, (StateMachine.ValueOrExceptionSink) this); return this::lookupPlatform; @@ -81,10 +87,10 @@ public void acceptValueOrException( if (value != null) { var pkg = ((PackageValue) value).getPackage(); try { - var label = platformKey.getLabel(); - var target = pkg.getTarget(label.getName()); + var target = pkg.getTarget(platformLabel.getName()); if (!PlatformLookupUtil.hasPlatformInfo(target)) { - sink.acceptPlatformInfoError(new InvalidPlatformException(label)); // validation failure + // validation failure + sink.acceptPlatformInfoError(new InvalidPlatformException(platformLabel)); return; } } catch (NoSuchTargetException e) { @@ -106,6 +112,13 @@ private StateMachine lookupPlatform(Tasks tasks) { return runAfter; } + // Create a configured target key with a dummy configuration. + ConfiguredTargetKey platformKey = + ConfiguredTargetKey.builder() + .setLabel(platformLabel) + .setConfigurationKey( + BuildConfigurationKey.withoutPlatformMapping(CommonOptions.EMPTY_OPTIONS)) + .build(); tasks.lookUp( platformKey, ConfiguredValueCreationException.class, this::acceptPlatformValueOrError); return this::retrievePlatformInfo; @@ -119,7 +132,7 @@ private void acceptPlatformValueOrError( return; } if (error != null) { - sink.acceptPlatformInfoError(new InvalidPlatformException(platformKey.getLabel(), error)); + sink.acceptPlatformInfoError(new InvalidPlatformException(platformLabel, error)); return; } throw new IllegalArgumentException("both value and error were null"); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/producers/TargetAndConfigurationProducer.java b/src/main/java/com/google/devtools/build/lib/analysis/producers/TargetAndConfigurationProducer.java index 3f8bbba7b5e091..37e841fbff4703 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/producers/TargetAndConfigurationProducer.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/producers/TargetAndConfigurationProducer.java @@ -285,13 +285,7 @@ public StateMachine step(Tasks tasks) throws InterruptedException { new PlatformConfiguration(preRuleTransitionKey.getConfigurationKey().getOptions()); tasks.enqueue( new PlatformInfoProducer( - ConfiguredTargetKey.builder() - .setLabel(platformConfiguration.getTargetPlatform()) - .setConfigurationKey( - unloadedToolchainContextsInputs - .targetToolchainContextKey() - .configurationKey()) - .build(), + platformConfiguration.getTargetPlatform(), (PlatformInfoProducer.ResultSink) this, this::computeConfigConditions)); } else {