BuildOptions#get returns null instead of throwing an exception.

Allows callers to do a single map lookup instead of calling `BuildOptions#contains` first. When removing `BuildConfigurationKeyCache`, the double lookup shows up in CPU profiles.

I audited all `BuildOptions#contains` calls and migrated to a single `BuildOptions#get` call where appropriate.

PiperOrigin-RevId: 673004425
Change-Id: Ia82ea33baa28384d796979777f3ba7e7b4bdd6b5

src/main/java/com/google/devtools/build/lib/analysis/PlatformConfiguration.java

@@ -39,9 +39,11 @@ public class PlatformConfiguration extends Fragment implements PlatformConfigura
   private final List<Map.Entry<RegexFilter, List<Label>>> targetFilterToAdditionalExecConstraints;
   private final RegexFilter toolchainResolutionDebugRegexFilter;
 
-  public PlatformConfiguration(BuildOptions buildOptions) {
-    PlatformOptions platformOptions = buildOptions.get(PlatformOptions.class);
+  public PlatformConfiguration(BuildOptions options) {
+    this(options.get(PlatformOptions.class));
+  }
 
+  public PlatformConfiguration(PlatformOptions platformOptions) {
     this.hostPlatform = platformOptions.hostPlatform;
     this.extraExecutionPlatforms = ImmutableList.copyOf(platformOptions.extraExecutionPlatforms);
     this.targetPlatform = platformOptions.computeTargetPlatform();

src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValue.java

@@ -255,15 +255,11 @@ private static ImmutableSortedMap<Class<? extends Fragment>, Fragment> getConfig
     this.buildOptions = buildOptions;
     this.mnemonic = mnemonic;
     this.options = buildOptions.get(CoreOptions.class);
-    PlatformOptions platformOptions = null;
-    if (buildOptions.contains(PlatformOptions.class)) {
-      platformOptions = buildOptions.get(PlatformOptions.class);
-    }
     this.outputDirectories =
         new OutputDirectories(
             directories,
             options,
-            platformOptions,
+            buildOptions.get(PlatformOptions.class),
             mnemonic,
             workspaceName,
             siblingRepositoryLayout);

src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java

@@ -14,7 +14,6 @@
 
 package com.google.devtools.build.lib.analysis.config;
 
-import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.collect.ImmutableMap.toImmutableMap;
 import static com.google.devtools.build.lib.skyframe.serialization.ImmutableMapCodecs.IMMUTABLE_MAP_CODEC;
 import static com.google.devtools.build.lib.skyframe.serialization.strings.UnsafeStringCodec.stringCodec;
@@ -125,10 +124,13 @@ public static BuildOptions of(Map<Label, Object> starlarkOptions) {
     return builder().addStarlarkOptions(starlarkOptions).build();
   }
 
-  /** Returns the actual instance of a FragmentOptions class. */
+  /**
+   * Returns the actual instance of a {@link FragmentOptions} class, or {@code null} if the options
+   * class is not present.
+   */
+  @Nullable
   public <T extends FragmentOptions> T get(Class<T> optionsClass) {
     FragmentOptions options = fragmentOptionsMap.get(optionsClass);
-    checkNotNull(options, "fragment options unavailable: %s", optionsClass);
     return optionsClass.cast(options);
   }
 

src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptionsView.java

@@ -16,6 +16,7 @@
 
 import com.google.common.base.Preconditions;
 import java.util.Set;
+import javax.annotation.Nullable;
 
 /**
  * Wrapper for {@link BuildOptions} that only permits {@link BuildOptions#get} calls to {@link
@@ -39,6 +40,7 @@ public BuildOptionsView(
    * Wrapper for {@link BuildOptions#get} that throws an {@link IllegalArgumentException} if the
    * given {@link FragmentOptions} isn't in the "permitted" set.
    */
+  @Nullable
   public <T extends FragmentOptions> T get(Class<T> optionsClass) {
     return options.get(checkFragment(optionsClass));
   }

src/main/java/com/google/devtools/build/lib/analysis/producers/BuildConfigurationKeyProducer.java

@@ -101,19 +101,20 @@ public StateMachine step(Tasks tasks) throws InterruptedException {
     }
 
     // Short-circuit if there are no platform options.
-    if (!options.contains(PlatformOptions.class)) {
+    var platformOptions = options.get(PlatformOptions.class);
+    if (platformOptions == null) {
       return finishConfigurationKeyProcessing(BuildConfigurationKey.create(options));
     }
 
-    List<Label> targetPlatforms = options.get(PlatformOptions.class).platforms;
+    List<Label> targetPlatforms = platformOptions.platforms;
     if (targetPlatforms.size() == 1) {
       // TODO: https://github.com/bazelbuild/bazel/issues/19807 - We define this flag to only use
       //  the first value and ignore any subsequent ones. Remove this check as part of cleanup.
       tasks.enqueue(
           new PlatformProducer(targetPlatforms.getFirst(), this, this::checkTargetPlatformFlags));
       return runAfter;
     } else {
-      return mergeFromPlatformMapping(tasks);
+      return mergeFromPlatformMapping(tasks, platformOptions);
     }
   }
 
@@ -126,12 +127,12 @@ private StateMachine checkTargetPlatformFlags(Tasks tasks) {
       BuildOptions updatedOptions = parsedFlags.get().mergeWith(options);
       return finishConfigurationKeyProcessing(BuildConfigurationKey.create(updatedOptions));
     } else {
-      return mergeFromPlatformMapping(tasks);
+      return mergeFromPlatformMapping(tasks, options.get(PlatformOptions.class));
     }
   }
 
-  private StateMachine mergeFromPlatformMapping(Tasks tasks) {
-    PathFragment platformMappingsPath = options.get(PlatformOptions.class).platformMappings;
+  private StateMachine mergeFromPlatformMapping(Tasks tasks, PlatformOptions platformOptions) {
+    PathFragment platformMappingsPath = platformOptions.platformMappings;
     tasks.lookUp(
         PlatformMappingValue.Key.create(platformMappingsPath),
         PlatformMappingException.class,

src/main/java/com/google/devtools/build/lib/analysis/producers/TargetAndConfigurationProducer.java

@@ -309,14 +309,12 @@ public UnloadedToolchainContextsInputs getUnloadedToolchainContextsInputs(
         return UnloadedToolchainContextsInputs.empty();
       }
 
-      if (!preRuleTransitionKey
-          .getConfigurationKey()
-          .getOptions()
-          .contains(PlatformOptions.class)) {
+      var platformOptions =
+          preRuleTransitionKey.getConfigurationKey().getOptions().get(PlatformOptions.class);
+      if (platformOptions == null) {
         return UnloadedToolchainContextsInputs.empty();
       }
-      PlatformConfiguration platformConfiguration =
-          new PlatformConfiguration(preRuleTransitionKey.getConfigurationKey().getOptions());
+      PlatformConfiguration platformConfiguration = new PlatformConfiguration(platformOptions);
       var defaultExecConstraintLabels =
           getExecutionPlatformConstraints(rule, platformConfiguration);
       var ruleClass = rule.getRuleClassObject();

src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java

@@ -190,9 +190,10 @@ private static BuildOptions maybeGetExecDefaults(
     //  1: --trim_test_configuration means the flags may not exist. Starlark logic needs to handle
     //     that possibility.
     //  2: --runs_per_test has a non-Starlark readable type.
-    if (fromOptions.contains(TestOptions.class)) {
+    var testOptions = fromOptions.get(TestOptions.class);
+    if (testOptions != null) {
       defaultBuilder.removeFragmentOptions(TestOptions.class);
-      defaultBuilder.addFragmentOptions(fromOptions.get(TestOptions.class));
+      defaultBuilder.addFragmentOptions(testOptions);
     }
     BuildOptions ans = defaultBuilder.build();
     if (fromOptions.get(CoreOptions.class).excludeDefinesFromExecConfig) {

src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java

@@ -334,10 +334,9 @@ public static class TestOptions extends FragmentOptions {
   private final ImmutableMap<TestSize, ImmutableMap<String, Double>> testResources;
 
   public TestConfiguration(BuildOptions buildOptions) {
-    this.shouldInclude = buildOptions.contains(TestOptions.class);
-    if (shouldInclude) {
-      TestOptions options = buildOptions.get(TestOptions.class);
-      this.options = options;
+    this.options = buildOptions.get(TestOptions.class);
+    if (options != null) {
+      this.shouldInclude = true;
       this.testTimeout = ImmutableMap.copyOf(options.testTimeout);
       ImmutableMap.Builder<TestSize, ImmutableMap<String, Double>> testResources =
           ImmutableMap.builderWithExpectedSize(TestSize.values().length);
@@ -350,7 +349,7 @@ public TestConfiguration(BuildOptions buildOptions) {
       }
       this.testResources = testResources.buildOrThrow();
     } else {
-      this.options = null;
+      this.shouldInclude = false;
       this.testTimeout = null;
       this.testResources = ImmutableMap.of();
     }

src/main/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionFactory.java

@@ -90,11 +90,11 @@ public ImmutableSet<Class<? extends FragmentOptions>> requiresOptionFragments()
     @Override
     public BuildOptions patch(BuildOptionsView originalOptions, EventHandler eventHandler)
         throws InterruptedException {
-      if (!originalOptions.contains(TestOptions.class)) {
+      var originalTestOptions = originalOptions.get(TestOptions.class);
+      if (originalTestOptions == null) {
         // nothing to do, already trimmed this fragment
         return originalOptions.underlying();
       }
-      TestOptions originalTestOptions = originalOptions.get(TestOptions.class);
       if (!originalTestOptions.trimTestConfiguration
           || (originalTestOptions.experimentalRetainTestConfigurationAcrossTestonly && testonly)) {
         // nothing to do, trimming is disabled

src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTaggedTrimmingTransitionFactory.java

@@ -62,9 +62,9 @@ public ImmutableSet<Class<? extends FragmentOptions>> requiresOptionFragments()
 
     @Override
     public BuildOptions patch(BuildOptionsView options, EventHandler eventHandler) {
-      if (!(options.contains(ConfigFeatureFlagOptions.class)
-          && options.get(ConfigFeatureFlagOptions.class)
-              .enforceTransitiveConfigsForConfigFeatureFlag)) {
+      var configFeatureFlagOptions = options.get(ConfigFeatureFlagOptions.class);
+      if (configFeatureFlagOptions == null
+          || !configFeatureFlagOptions.enforceTransitiveConfigsForConfigFeatureFlag) {
         return options.underlying();
       }
       BuildOptions toOptions = FeatureFlagValue.trimFlagValues(options.underlying(), flags);

src/main/java/com/google/devtools/build/lib/rules/config/FeatureFlagValue.java

@@ -75,8 +75,9 @@ static BuildOptions replaceFlagValues(BuildOptions original, Map<Label, String>
     }
     result.addStarlarkOptions(newValueObjects.buildOrThrow());
     BuildOptions builtResult = result.build();
-    if (builtResult.contains(ConfigFeatureFlagOptions.class)) {
-      builtResult.get(ConfigFeatureFlagOptions.class).allFeatureFlagValuesArePresent = true;
+    var configFeatureFlagOptions = builtResult.get(ConfigFeatureFlagOptions.class);
+    if (configFeatureFlagOptions != null) {
+      configFeatureFlagOptions.allFeatureFlagValuesArePresent = true;
     }
     return builtResult;
   }
@@ -90,11 +91,10 @@ static BuildOptions trimFlagValues(BuildOptions original, Set<Label> availableFl
     Set<Label> seenFlags = new LinkedHashSet<>();
     Set<Label> flagsToTrim = new LinkedHashSet<>();
     Map<Label, Object> unknownFlagsToAdd = new LinkedHashMap<>();
-    boolean changeAllValuesPresentOption = false;
-    if (original.contains(ConfigFeatureFlagOptions.class)) {
-      changeAllValuesPresentOption =
-          original.get(ConfigFeatureFlagOptions.class).allFeatureFlagValuesArePresent;
-    }
+    var originalConfigFeatureFlagOptions = original.get(ConfigFeatureFlagOptions.class);
+    boolean changeAllValuesPresentOption =
+        originalConfigFeatureFlagOptions != null
+            && originalConfigFeatureFlagOptions.allFeatureFlagValuesArePresent;
 
     // What do we need to change?
     original.getStarlarkOptions().entrySet().stream()
@@ -117,8 +117,9 @@ static BuildOptions trimFlagValues(BuildOptions original, Set<Label> availableFl
     flagsToTrim.forEach(trimmedFlag -> result.removeStarlarkOption(trimmedFlag));
     unknownFlagsToAdd.forEach((flag, value) -> result.addStarlarkOption(flag, value));
     BuildOptions builtResult = result.build();
-    if (builtResult.contains(ConfigFeatureFlagOptions.class)) {
-      builtResult.get(ConfigFeatureFlagOptions.class).allFeatureFlagValuesArePresent = false;
+    var builtConfigFeatureFlagOptions = builtResult.get(ConfigFeatureFlagOptions.class);
+    if (builtConfigFeatureFlagOptions != null) {
+      builtConfigFeatureFlagOptions.allFeatureFlagValuesArePresent = false;
     }
     return builtResult;
   }

src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java

@@ -3124,8 +3124,8 @@ private void maybeDropGenQueryDep(SkyValue newValue, GroupedDeps directDeps) {
         return;
       }
       BuildOptions options = t.getConfigurationKey().getOptions();
-      if (!options.contains(GenQueryOptions.class)
-          || !options.get(GenQueryOptions.class).skipTtvs) {
+      var genQueryOptions = options.get(GenQueryOptions.class);
+      if (genQueryOptions == null || !genQueryOptions.skipTtvs) {
         return;
       }
       for (SkyKey key : directDeps.getAllElementsAsIterable()) {

src/main/java/com/google/devtools/build/lib/skyframe/config/BuildConfigurationFunction.java

@@ -111,10 +111,7 @@ private static Optional<BuildOptions> getBaselineOptions(
       return Optional.empty();
     }
 
-    PlatformOptions platformOptions = null;
-    if (targetOptions.contains(PlatformOptions.class)) {
-      platformOptions = targetOptions.get(PlatformOptions.class);
-    }
+    var platformOptions = targetOptions.get(PlatformOptions.class);
 
     // Determine whether this is part of the exec transition, or if we need to calculate a target
     // platform.

src/main/java/com/google/devtools/build/lib/skyframe/config/PlatformMappingValue.java

@@ -201,20 +201,20 @@ public BuildOptions map(BuildOptions original) throws OptionsParsingException {
   }
 
   private BuildOptions computeMapping(BuildOptions originalOptions) throws OptionsParsingException {
-
     if (originalOptions.hasNoConfig()) {
       // The empty configuration (produced by NoConfigTransition) is terminal: it'll never change.
       return originalOptions;
     }
 
+    var platformOptions = originalOptions.get(PlatformOptions.class);
     checkArgument(
-        originalOptions.contains(PlatformOptions.class),
+        platformOptions != null,
         "When using platform mappings, all configurations must contain platform options");
 
     BuildOptions modifiedOptions = null;
 
-    if (!originalOptions.get(PlatformOptions.class).platforms.isEmpty()) {
-      List<Label> platforms = originalOptions.get(PlatformOptions.class).platforms;
+    if (!platformOptions.platforms.isEmpty()) {
+      List<Label> platforms = platformOptions.platforms;
 
       // Platform mapping only supports a single target platform, others are ignored.
       Label targetPlatform = Iterables.getFirst(platforms, null);
@@ -240,7 +240,7 @@ private BuildOptions computeMapping(BuildOptions originalOptions) throws Options
       }
 
       if (!mappingFound) {
-        Label targetPlatform = originalOptions.get(PlatformOptions.class).computeTargetPlatform();
+        Label targetPlatform = platformOptions.computeTargetPlatform();
         modifiedOptions = originalOptions.clone();
         modifiedOptions.get(PlatformOptions.class).platforms = ImmutableList.of(targetPlatform);
       }

src/test/java/com/google/devtools/build/lib/analysis/config/ExecutionTransitionFactoryTest.java

@@ -68,9 +68,7 @@ public void executionTransition() throws Exception {
     assertThat(result).isNotNull();
     assertThat(result).isNotSameInstanceAs(options);
 
-    assertThat(result.contains(CoreOptions.class)).isTrue();
     assertThat(result.get(CoreOptions.class).isExec).isTrue();
-    assertThat(result.contains(PlatformOptions.class)).isTrue();
     assertThat(result.get(PlatformOptions.class).platforms).containsExactly(EXECUTION_PLATFORM);
   }
 

src/test/java/com/google/devtools/build/lib/analysis/producers/BuildConfigurationKeyMapProducerTest.java

@@ -115,7 +115,6 @@ public void createKey() throws Exception {
     BuildConfigurationKey result = fetch(baseOptions);
 
     assertThat(result).isNotNull();
-    assertThat(result.getOptions().contains(DummyTestOptions.class)).isTrue();
     assertThat(result.getOptions().get(DummyTestOptions.class).internalOption)
         .isEqualTo("from_cmd");
   }
@@ -159,7 +158,6 @@ public void createKey_platformMapping() throws Exception {
     BuildConfigurationKey result = fetch(baseOptions);
 
     assertThat(result).isNotNull();
-    assertThat(result.getOptions().contains(DummyTestOptions.class)).isTrue();
     assertThat(result.getOptions().get(DummyTestOptions.class).internalOption)
         .isEqualTo("from_mapping_changed");
   }
@@ -213,7 +211,6 @@ public void createKey_platformFlags() throws Exception {
     BuildConfigurationKey result = fetch(baseOptions);
 
     assertThat(result).isNotNull();
-    assertThat(result.getOptions().contains(DummyTestOptions.class)).isTrue();
     assertThat(result.getOptions().get(DummyTestOptions.class).internalOption)
         .isEqualTo("from_platform");
   }
@@ -237,7 +234,6 @@ public void createKey_platformFlags_override() throws Exception {
     BuildConfigurationKey result = fetch(baseOptions);
 
     assertThat(result).isNotNull();
-    assertThat(result.getOptions().contains(DummyTestOptions.class)).isTrue();
     assertThat(result.getOptions().get(DummyTestOptions.class).option).isEqualTo("from_platform");
   }
 
@@ -262,7 +258,6 @@ public void createKey_platformFlags_accumulate() throws Exception {
     BuildConfigurationKey result = fetch(baseOptions);
 
     assertThat(result).isNotNull();
-    assertThat(result.getOptions().contains(DummyTestOptions.class)).isTrue();
     assertThat(result.getOptions().get(DummyTestOptions.class).accumulating)
         .containsExactly("from_cli", "from_platform")
         .inOrder();
@@ -324,7 +319,6 @@ public void createKey_platformFlags_overridesMapping() throws Exception {
     BuildConfigurationKey result = fetch(baseOptions);
 
     assertThat(result).isNotNull();
-    assertThat(result.getOptions().contains(DummyTestOptions.class)).isTrue();
     assertThat(result.getOptions().get(DummyTestOptions.class).internalOption)
         .isEqualTo("from_platform");
   }