Remove defaultBuildOptions from PlatformMappingValue.

It really only needs a list of options classes for parsing. Additionally, instead of passing it in, just store this list as a field. There shouldn't be any penalty for doing this, as equality checks will be fast since it's always the same instance in practice, and we don't serialize `PlatformMappingValue`.

After this, we can tear out `defaultBuildOptions` entirely. `PlatformMappingValue` was the last one using it.

PiperOrigin-RevId: 372134563

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

@@ -95,19 +95,16 @@ private ValueMissingException() {
   private final SkyFunction.Environment env;
   private final TargetAndConfiguration ctgValue;
   private final BuildConfiguration hostConfiguration;
-  private final BuildOptions defaultBuildOptions;
   private final ImmutableMap<Label, ConfigMatchingProvider> configConditions;
 
   public ConfigurationResolver(
       SkyFunction.Environment env,
       TargetAndConfiguration ctgValue,
       BuildConfiguration hostConfiguration,
-      BuildOptions defaultBuildOptions,
       ImmutableMap<Label, ConfigMatchingProvider> configConditions) {
     this.env = env;
     this.ctgValue = ctgValue;
     this.hostConfiguration = hostConfiguration;
-    this.defaultBuildOptions = defaultBuildOptions;
     this.configConditions = configConditions;
   }
 
@@ -249,7 +246,7 @@ private ImmutableList<Dependency> resolveGenericTransition(
         String transitionKey = optionsEntry.getKey();
         BuildConfigurationValue.Key buildConfigurationValueKey =
             BuildConfigurationValue.keyWithPlatformMapping(
-                platformMappingValue, defaultBuildOptions, depFragments, optionsEntry.getValue());
+                platformMappingValue, depFragments, optionsEntry.getValue());
         configurationKeys.put(transitionKey, buildConfigurationValueKey);
       }
     } catch (OptionsParsingException e) {

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

@@ -37,7 +37,6 @@
 import com.google.devtools.build.lib.analysis.TargetAndConfiguration;
 import com.google.devtools.build.lib.analysis.ToolchainCollection;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
-import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.ConfigConditions;
 import com.google.devtools.build.lib.analysis.config.DependencyEvaluationException;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
@@ -83,25 +82,23 @@
 /**
  * The Skyframe function that generates aspects.
  *
- * This class, together with {@link ConfiguredTargetFunction} drives the analysis phase. For more
+ * <p>This class, together with {@link ConfiguredTargetFunction} drives the analysis phase. For more
  * information, see {@link com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory}.
  *
- * {@link AspectFunction} takes a SkyKey containing an {@link AspectKey} [a tuple of
- * (target label, configurations, aspect class and aspect parameters)],
- * loads an {@link Aspect} from aspect class and aspect parameters,
- * gets a {@link ConfiguredTarget} for label and configurations, and then creates
- * a {@link ConfiguredAspect} for a given {@link AspectKey}.
+ * <p>{@link AspectFunction} takes a SkyKey containing an {@link AspectKey} [a tuple of (target
+ * label, configurations, aspect class and aspect parameters)], loads an {@link Aspect} from aspect
+ * class and aspect parameters, gets a {@link ConfiguredTarget} for label and configurations, and
+ * then creates a {@link ConfiguredAspect} for a given {@link AspectKey}.
  *
- * See {@link com.google.devtools.build.lib.packages.AspectClass} documentation
- * for an overview of aspect-related classes
+ * <p>See {@link com.google.devtools.build.lib.packages.AspectClass} documentation for an overview
+ * of aspect-related classes
  *
  * @see com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory
  * @see com.google.devtools.build.lib.packages.AspectClass
  */
-public final class AspectFunction implements SkyFunction {
+final class AspectFunction implements SkyFunction {
   private final BuildViewProvider buildViewProvider;
   private final RuleClassProvider ruleClassProvider;
-  private final BuildOptions defaultBuildOptions;
   /**
    * Indicates whether the set of packages transitively loaded for a given {@link AspectValue} will
    * be needed for package root resolution later in the build. If not, they are not collected and
@@ -112,13 +109,11 @@ public final class AspectFunction implements SkyFunction {
   AspectFunction(
       BuildViewProvider buildViewProvider,
       RuleClassProvider ruleClassProvider,
-      boolean storeTransitivePackagesForPackageRootResolution,
-      BuildOptions defaultBuildOptions) {
+      boolean storeTransitivePackagesForPackageRootResolution) {
     this.buildViewProvider = buildViewProvider;
     this.ruleClassProvider = ruleClassProvider;
     this.storeTransitivePackagesForPackageRootResolution =
         storeTransitivePackagesForPackageRootResolution;
-    this.defaultBuildOptions = defaultBuildOptions;
   }
 
   /**
@@ -426,8 +421,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
                 ruleClassProvider,
                 view.getHostConfiguration(originalTargetAndAspectConfiguration.getConfiguration()),
                 transitivePackagesForPackageRootResolution,
-                transitiveRootCauses,
-                defaultBuildOptions);
+                transitiveRootCauses);
       } catch (ConfiguredValueCreationException e) {
         throw new AspectCreationException(
             e.getMessage(), key.getLabel(), aspectConfiguration, e.getDetailedExitCode());

src/main/java/com/google/devtools/build/lib/skyframe/BUILD

@@ -238,6 +238,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/analysis:config/core_options",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_class_set",
+        "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_options",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/host_transition",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/invalid_configuration_exception",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/configuration_transition",
@@ -274,7 +275,6 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/collect/nestedset",
         "//src/main/java/com/google/devtools/build/lib/concurrent",
         "//src/main/java/com/google/devtools/build/lib/events",
-        "//src/main/java/com/google/devtools/build/lib/exec:execution_options",
         "//src/main/java/com/google/devtools/build/lib/io:file_symlink_cycle_uniqueness_function",
         "//src/main/java/com/google/devtools/build/lib/io:file_symlink_exception",
         "//src/main/java/com/google/devtools/build/lib/io:file_symlink_infinite_expansion_uniqueness_function",
@@ -964,6 +964,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/build_options",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_class_set",
+        "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_options",
         "//src/main/java/com/google/devtools/build/lib/analysis:platform_options",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/concurrent",

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

@@ -52,20 +52,16 @@ public BuildConfiguration getConfiguration() {
    *
    * @param platformMappingValue sky value that can transform a configuration key based on a
    *     platform mapping
-   * @param defaultBuildOptions set of native build options without modifications based on parsing
-   *     flags
    * @param fragments set of options fragments this configuration should cover
    * @param options the desired configuration
    * @throws OptionsParsingException if the platform mapping cannot be parsed
    */
   public static Key keyWithPlatformMapping(
       PlatformMappingValue platformMappingValue,
-      BuildOptions defaultBuildOptions,
       FragmentClassSet fragments,
       BuildOptions options)
       throws OptionsParsingException {
-    return platformMappingValue.map(
-        keyWithoutPlatformMapping(fragments, options), defaultBuildOptions);
+    return platformMappingValue.map(keyWithoutPlatformMapping(fragments, options));
   }
 
   /**

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

@@ -136,7 +136,6 @@ public static ConfiguredValueCreationException asConfiguredValueCreationExceptio
   private final BuildViewProvider buildViewProvider;
   private final RuleClassProvider ruleClassProvider;
   private final Semaphore cpuBoundSemaphore;
-  private final BuildOptions defaultBuildOptions;
   @Nullable private final ConfiguredTargetProgressReceiver configuredTargetProgress;
 
   /**
@@ -154,15 +153,13 @@ public static ConfiguredValueCreationException asConfiguredValueCreationExceptio
       Semaphore cpuBoundSemaphore,
       boolean storeTransitivePackagesForPackageRootResolution,
       boolean shouldUnblockCpuWorkWhenFetchingDeps,
-      BuildOptions defaultBuildOptions,
       @Nullable ConfiguredTargetProgressReceiver configuredTargetProgress) {
     this.buildViewProvider = buildViewProvider;
     this.ruleClassProvider = ruleClassProvider;
     this.cpuBoundSemaphore = cpuBoundSemaphore;
     this.storeTransitivePackagesForPackageRootResolution =
         storeTransitivePackagesForPackageRootResolution;
     this.shouldUnblockCpuWorkWhenFetchingDeps = shouldUnblockCpuWorkWhenFetchingDeps;
-    this.defaultBuildOptions = defaultBuildOptions;
     this.configuredTargetProgress = configuredTargetProgress;
   }
 
@@ -314,8 +311,7 @@ public SkyValue compute(SkyKey key, Environment env) throws ConfiguredTargetFunc
               ruleClassProvider,
               view.getHostConfiguration(configuration),
               transitivePackagesForPackageRootResolution,
-              transitiveRootCauses,
-              defaultBuildOptions);
+              transitiveRootCauses);
       if (!transitiveRootCauses.isEmpty()) {
         NestedSet<Cause> causes = transitiveRootCauses.build();
         throw new ConfiguredTargetFunctionException(
@@ -601,9 +597,6 @@ public static ImmutableSet<Label> getExecutionPlatformConstraints(
    * @param hostConfiguration the host configuration. There's a noticeable performance hit from
    *     instantiating this on demand for every dependency that wants it, so it's best to compute
    *     the host configuration as early as possible and pass this reference to all consumers
-   * @param defaultBuildOptions the default build options provided by the server; these are used to
-   *     create diffs for {@link BuildConfigurationValue.Key}s to prevent storing the entire
-   *     BuildOptions object.
    */
   @Nullable
   static OrderedSetMultimap<DependencyKind, ConfiguredTargetAndData> computeDependencies(
@@ -617,8 +610,7 @@ static OrderedSetMultimap<DependencyKind, ConfiguredTargetAndData> computeDepend
       RuleClassProvider ruleClassProvider,
       BuildConfiguration hostConfiguration,
       @Nullable NestedSetBuilder<Package> transitivePackagesForPackageRootResolution,
-      NestedSetBuilder<Cause> transitiveRootCauses,
-      BuildOptions defaultBuildOptions)
+      NestedSetBuilder<Cause> transitiveRootCauses)
       throws DependencyEvaluationException, ConfiguredValueCreationException,
           AspectCreationException, InterruptedException {
     // Create the map from attributes to set of (target, transition) pairs.
@@ -648,8 +640,7 @@ static OrderedSetMultimap<DependencyKind, ConfiguredTargetAndData> computeDepend
     // Trim each dep's configuration so it only includes the fragments needed by its transitive
     // closure.
     ConfigurationResolver configResolver =
-        new ConfigurationResolver(
-            env, ctgValue, hostConfiguration, defaultBuildOptions, configConditions);
+        new ConfigurationResolver(env, ctgValue, hostConfiguration, configConditions);
     OrderedSetMultimap<DependencyKind, Dependency> depValueNames =
         configResolver.resolveConfigurations(initialDependencies);
 

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

@@ -14,6 +14,7 @@
 
 package com.google.devtools.build.lib.skyframe;
 
+import static com.google.common.base.Preconditions.checkNotNull;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.google.common.annotations.VisibleForTesting;
@@ -24,7 +25,7 @@
 import com.google.common.collect.PeekingIterator;
 import com.google.devtools.build.lib.actions.FileValue;
 import com.google.devtools.build.lib.actions.MissingInputFileException;
-import com.google.devtools.build.lib.analysis.config.BuildOptions;
+import com.google.devtools.build.lib.analysis.config.FragmentOptions;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
@@ -39,9 +40,7 @@
 import com.google.devtools.build.skyframe.SkyFunctionException;
 import com.google.devtools.build.skyframe.SkyKey;
 import java.io.IOException;
-import java.util.Collection;
 import java.util.List;
-import java.util.Map;
 import javax.annotation.Nullable;
 import net.starlark.java.syntax.Location;
 
@@ -50,10 +49,15 @@
  * parses them for use in a {@link PlatformMappingValue}.
  *
  * <p>Note that this class only parses the mapping-file specific format, parsing (and validation) of
- * flags contained therein is left to the invocation of {@link
- * PlatformMappingValue#map(BuildConfigurationValue.Key, BuildOptions)}.
+ * flags contained therein is left to the invocation of {@link PlatformMappingValue#map}.
  */
-public class PlatformMappingFunction implements SkyFunction {
+final class PlatformMappingFunction implements SkyFunction {
+
+  private final ImmutableList<Class<? extends FragmentOptions>> optionsClasses;
+
+  PlatformMappingFunction(ImmutableList<Class<? extends FragmentOptions>> optionsClasses) {
+    this.optionsClasses = checkNotNull(optionsClasses);
+  }
 
   @Nullable
   @Override
@@ -99,7 +103,7 @@ public PlatformMappingValue compute(SkyKey skyKey, Environment env)
         throw new PlatformMappingException(e, SkyFunctionException.Transience.TRANSIENT);
       }
 
-      return parse(lines).toPlatformMappingValue();
+      return parse(lines).toPlatformMappingValue(optionsClasses);
     }
 
     if (!platformMappingKey.wasExplicitlySetByUser()) {
@@ -157,8 +161,8 @@ static Mappings parse(List<String> lines) throws PlatformMappingException {
       throw parsingException("Expected 'platforms:' or 'flags:' but got " + it.peek());
     }
 
-    Map<Label, Collection<String>> platformsToFlags = ImmutableMap.of();
-    Map<Collection<String>, Label> flagsToPlatforms = ImmutableMap.of();
+    ImmutableMap<Label, ImmutableSet<String>> platformsToFlags = ImmutableMap.of();
+    ImmutableMap<ImmutableSet<String>, Label> flagsToPlatforms = ImmutableMap.of();
 
     if (it.peek().equalsIgnoreCase("platforms:")) {
       it.next();
@@ -179,12 +183,12 @@ static Mappings parse(List<String> lines) throws PlatformMappingException {
     return new Mappings(platformsToFlags, flagsToPlatforms);
   }
 
-  private static ImmutableMap<Label, Collection<String>> readPlatformsToFlags(
+  private static ImmutableMap<Label, ImmutableSet<String>> readPlatformsToFlags(
       PeekingIterator<String> it) throws PlatformMappingException {
-    ImmutableMap.Builder<Label, Collection<String>> platformsToFlags = ImmutableMap.builder();
+    ImmutableMap.Builder<Label, ImmutableSet<String>> platformsToFlags = ImmutableMap.builder();
     while (it.hasNext() && !it.peek().equalsIgnoreCase("flags:")) {
       Label platform = readPlatform(it);
-      Collection<String> flags = readFlags(it);
+      ImmutableSet<String> flags = readFlags(it);
       platformsToFlags.put(platform, flags);
     }
 
@@ -196,11 +200,11 @@ private static ImmutableMap<Label, Collection<String>> readPlatformsToFlags(
     }
   }
 
-  private static ImmutableMap<Collection<String>, Label> readFlagsToPlatforms(
+  private static ImmutableMap<ImmutableSet<String>, Label> readFlagsToPlatforms(
       PeekingIterator<String> it) throws PlatformMappingException {
-    ImmutableMap.Builder<Collection<String>, Label> flagsToPlatforms = ImmutableMap.builder();
+    ImmutableMap.Builder<ImmutableSet<String>, Label> flagsToPlatforms = ImmutableMap.builder();
     while (it.hasNext() && it.peek().startsWith("--")) {
-      Collection<String> flags = readFlags(it);
+      ImmutableSet<String> flags = readFlags(it);
       Label platform = readPlatform(it);
       flagsToPlatforms.put(flags, platform);
     }
@@ -259,19 +263,20 @@ private static PlatformMappingException parsingException(String message, Excepti
    * Simple data holder to make testing easier. Only for use internal to this file/tests thereof.
    */
   @VisibleForTesting
-  static class Mappings {
-    final Map<Label, Collection<String>> platformsToFlags;
-    final Map<Collection<String>, Label> flagsToPlatforms;
+  static final class Mappings {
+    final ImmutableMap<Label, ImmutableSet<String>> platformsToFlags;
+    final ImmutableMap<ImmutableSet<String>, Label> flagsToPlatforms;
 
     Mappings(
-        Map<Label, Collection<String>> platformsToFlags,
-        Map<Collection<String>, Label> flagsToPlatforms) {
+        ImmutableMap<Label, ImmutableSet<String>> platformsToFlags,
+        ImmutableMap<ImmutableSet<String>, Label> flagsToPlatforms) {
       this.platformsToFlags = platformsToFlags;
       this.flagsToPlatforms = flagsToPlatforms;
     }
 
-    PlatformMappingValue toPlatformMappingValue() {
-      return new PlatformMappingValue(platformsToFlags, flagsToPlatforms);
+    PlatformMappingValue toPlatformMappingValue(
+        ImmutableList<Class<? extends FragmentOptions>> optionsClasses) {
+      return new PlatformMappingValue(platformsToFlags, flagsToPlatforms, optionsClasses);
     }
   }