Update PlatformInfoProducer to not use a configuration key.

All platforms are non-configured (since bazelbuild@87fb462), so we can use an empty configuration and de-duplicate requests. Work towards platform-based flags: bazelbuild#19409. PiperOrigin-RevId: 569577077 Change-Id: Iea70d2c1484df17bc12925317415fc32cbd90ccc
bazel-io · Sep 29, 2023 · d6254dc · d6254dc
1 parent 0626570
commit d6254dc
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 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
@@ -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/...evtools/build/lib/analysis/producers/DependencyContextProducerWithCompatibilityCheck.java b/...evtools/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
@@ -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.
+ *
+ * <p>Since platforms do not rely on the configuration, this uses a dummy blank configuration to
+ * help reduce the number of skyframe edges created.
  *
  * <p>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<NoSuchPackageException>) 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/...java/com/google/devtools/build/lib/analysis/producers/TargetAndConfigurationProducer.java b/...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 {