Add diff_against_dynamic_baseline option to experimental_output_directory_naming_scheme.

To support, a new SkyFunction is introduced, BaselineOptionsFunction, with corresponding SkyValue and SkyKey. Also, a new function ExecutionTransitionFactory.createTransition was added to immediately create an ExecutionTransition.

When --experimental_output_directory_naming_scheme=diff_against_dynamic_baseline, the behavior is similar to diff_against_baseline, except when "is Exec" is true (that is, an ExecutionTransition has previously been applied), the baseline is chosen to be result of the ExecutionTransition being applied to the normal baseline.

This is meant to resolve an issue with performance when using remote execution engines that collate requests from multiple users. They were seeing varying output paths for actions in an execution configuration depending on how much 'resetting' the exec transition was doing because of varying user commandlines.

This relates to #14023 and #18002

PiperOrigin-RevId: 537097551
Change-Id: I72966406b7b8af0174a81b638bf0d1fb62466653

Author: Googler

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

@@ -1352,6 +1352,21 @@ java_library(
     ],
 )
 
+java_library(
+    name = "config/transitions/baseline_options_value",
+    srcs = [
+        "config/transitions/BaselineOptionsValue.java",
+    ],
+    deps = [
+        ":config/build_options",
+        "//src/main/java/com/google/devtools/build/lib/concurrent:thread_safety",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions",
+        "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
+        "//third_party:auto_value",
+        "//third_party:error_prone_annotations",
+    ],
+)
+
 # TODO(b/144899336): This should be analysis/actions/BUILD
 java_library(
     name = "actions/abstract_file_write_action",

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

@@ -563,7 +563,9 @@ public enum OutputDirectoryNamingScheme {
     /** Use `affected by starlark transition` to track configuration changes */
     LEGACY,
     /** Produce name based on diff from some baseline BuildOptions (usually top-level) */
-    DIFF_AGAINST_BASELINE
+    DIFF_AGAINST_BASELINE,
+    /** Like DIFF_AGAINST_BASELINE, but compare against post-exec baseline if isExec is set. */
+    DIFF_AGAINST_DYNAMIC_BASELINE
   }
 
   /** Converter for the {@code --experimental_output_directory_naming_scheme} options. */
@@ -589,6 +591,17 @@ public OutputDirectoryNamingSchemeConverter() {
               + " for all configuration, by diffing against the top-level configuration.")
   public OutputDirectoryNamingScheme outputDirectoryNamingScheme;
 
+  public boolean useBaselineForOutputDirectoryNamingScheme() {
+    switch (outputDirectoryNamingScheme) {
+      case DIFF_AGAINST_BASELINE:
+      case DIFF_AGAINST_DYNAMIC_BASELINE:
+        return true;
+      case LEGACY:
+        return false;
+    }
+    throw new IllegalStateException("unreachable");
+  }
+
   @Option(
       name = "is exec configuration",
       defaultValue = "false",

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

@@ -36,6 +36,11 @@
  * {@link TransitionFactory} implementation which creates a {@link PatchTransition} which will
  * transition to a configuration suitable for building dependencies for the execution platform of
  * the depending target.
+ *
+ * <p>Note that execGroup is not directly consumed by the involved transition but instead stored
+ * here. Instead, the rule definition stores it in this factory. Then, toolchain resolution extracts
+ * and consumes it to store an execution platform in attrs. Finally, the execution platform is read
+ * by the factory to create the transition.
  */
 public class ExecutionTransitionFactory
     implements TransitionFactory<AttributeTransitionData>, ExecTransitionFactoryApi {
@@ -48,6 +53,11 @@ public static ExecutionTransitionFactory createFactory() {
     return new ExecutionTransitionFactory(DEFAULT_EXEC_GROUP_NAME);
   }
 
+  /** Returns a new {@link ExecutionTransition} immediately. */
+  public static PatchTransition createTransition(@Nullable Label executionPlatform) {
+    return new ExecutionTransition(executionPlatform);
+  }
+
   /**
    * Returns a new {@link ExecutionTransitionFactory} for the given {@link
    * com.google.devtools.build.lib.packages.ExecGroup}.
@@ -104,6 +114,10 @@ public String getName() {
 
     @Override
     public ImmutableSet<Class<? extends FragmentOptions>> requiresOptionFragments() {
+      // This is technically a lie since the call to underlying().createExecOptions is transitively
+      // reading and potentially modifying all fragments. There is currently no way for the
+      // transition to actually list all fragments like this and thus only lists the ones that are
+      // directly being read here. Note that this transition is exceptional in its implementation.
       return FRAGMENTS;
     }
 

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

@@ -0,0 +1,74 @@
+// Copyright 2023 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.analysis.config.transitions;
+
+import com.google.auto.value.AutoValue;
+import com.google.devtools.build.lib.analysis.config.BuildOptions;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
+import com.google.devtools.build.lib.skyframe.SkyFunctions;
+import com.google.devtools.build.skyframe.SkyFunctionName;
+import com.google.devtools.build.skyframe.SkyKey;
+import com.google.devtools.build.skyframe.SkyValue;
+import com.google.errorprone.annotations.CheckReturnValue;
+
+/**
+ * This contains the baseline options to compare against when constructing output paths.
+ *
+ * <p>When constructing the output mnemonic as part of making a {@link BuildConfigurationValue} and
+ * the selected naming scheme is to diff against a baseline, this function returns the baseline to
+ * use for that comparison. Differences in options between the given option and this baseline will
+ * then be used to append a deconflicting ST-hash to the output mnemonic.
+ *
+ * <p>The afterExecTransition option in the key will apply the exec transition to the usual
+ * baseline. It is expected that this is set whenever the given options have isExec set (and thus an
+ * exec transition has already been applied to those options). The expectation here is that, as the
+ * exec transition particularly sets many options, comparing against a post-exec baseline will yield
+ * fewer diffenences. Note that some indicator must be added to the mnemonic (e.g. -exec-) in order
+ * to deconflict for similar options where isExec is not set.
+ */
+@CheckReturnValue
+@Immutable
+@ThreadSafe
+@AutoValue
+public abstract class BaselineOptionsValue implements SkyValue {
+  public abstract BuildOptions toOptions();
+
+  public static BaselineOptionsValue create(BuildOptions toOptions) {
+    return new AutoValue_BaselineOptionsValue(toOptions);
+  }
+
+  public static Key key(boolean afterExecTransition) {
+    return Key.create(afterExecTransition);
+  }
+
+  /** {@link SkyKey} implementation used for {@link BaselineOptionsValue}. */
+  @CheckReturnValue
+  @Immutable
+  @ThreadSafe
+  @AutoValue
+  public abstract static class Key implements SkyKey {
+    public abstract boolean afterExecTransition();
+
+    @Override
+    public SkyFunctionName functionName() {
+      return SkyFunctions.BASELINE_OPTIONS;
+    }
+
+    static Key create(boolean afterExecTransition) {
+      return new AutoValue_BaselineOptionsValue_Key(afterExecTransition);
+    }
+  }
+}

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

@@ -29,6 +29,7 @@ java_library(
         "ActionOutputDirectoryHelper.java",
         "AspectCompletor.java",
         "AspectFunction.java",
+        "BaselineOptionsFunction.java",
         "BazelSkyframeExecutorConstants.java",
         "BuildConfigurationFunction.java",
         "BuildInfoCollectionFunction.java",
@@ -234,13 +235,15 @@ 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/core_options",
+        "//src/main/java/com/google/devtools/build/lib/analysis:config/execution_transition_factory",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_factory",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_options",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/invalid_configuration_exception",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/optioninfo",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/starlark_defined_config_transition",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/starlark_transition_cache",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/toolchain_type_requirement",
+        "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/baseline_options_value",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/configuration_transition",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/no_transition",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/null_transition",

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

@@ -0,0 +1,90 @@
+// Copyright 2023 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.skyframe;
+
+import com.google.devtools.build.lib.analysis.PlatformOptions;
+import com.google.devtools.build.lib.analysis.config.BuildOptions;
+import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory;
+import com.google.devtools.build.lib.analysis.config.transitions.BaselineOptionsValue;
+import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
+import com.google.devtools.build.lib.analysis.config.transitions.TransitionUtil;
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.skyframe.SkyFunction;
+import com.google.devtools.build.skyframe.SkyFunctionException;
+import com.google.devtools.build.skyframe.SkyKey;
+import com.google.devtools.build.skyframe.SkyValue;
+import com.google.devtools.common.options.OptionsParsingException;
+import javax.annotation.Nullable;
+
+/** A builder for {@link BaselineOptionsValue} instances. */
+public final class BaselineOptionsFunction implements SkyFunction {
+  @Override
+  @Nullable
+  public SkyValue compute(SkyKey skyKey, Environment env)
+      throws InterruptedException, BaselineOptionsFunctionException {
+    BaselineOptionsValue.Key key = (BaselineOptionsValue.Key) skyKey.argument();
+
+    BuildOptions rawBaselineOptions = PrecomputedValue.BASELINE_CONFIGURATION.get(env);
+
+    // Some test infrastructure only creates mock or partial top-level BuildOptions such that
+    // PlatformOptions or even CoreOptions might not be included.
+    // In that case, is not worth doing any special processing of the baseline.
+    if (rawBaselineOptions.hasNoConfig()) {
+      return BaselineOptionsValue.create(rawBaselineOptions);
+    }
+
+    // Herein lies a hack to apply platform mappings to the baseline options.
+    // TODO(blaze-configurability-team): this should become unnecessary once --platforms is marked
+    //   as EXPLICIT_IN_OUTPUT_PATH
+    PlatformMappingValue platformMappingValue =
+        (PlatformMappingValue)
+            env.getValue(
+                PlatformMappingValue.Key.create(
+                    rawBaselineOptions.get(PlatformOptions.class).platformMappings));
+    if (platformMappingValue == null) {
+      return null;
+    }
+    try {
+      BuildOptions mappedBaselineOptions =
+          BuildConfigurationKey.withPlatformMapping(platformMappingValue, rawBaselineOptions)
+              .getOptions();
+
+      if (key.afterExecTransition()) {
+        // A null executionPlatform actually skips transition application so need some value here.
+        // 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
+        // TODO(twigg): This can eventually be replaced by the actual exec platform once
+        //   platforms is explicitly in the output path (with the garbage value as a fallback).
+        PatchTransition execTransition =
+            ExecutionTransitionFactory.createTransition(
+                Label.parseCanonicalUnchecked(
+                    "//this_is_a_faked_exec_platform_for_blaze_internals"));
+        BuildOptions toOptions =
+            execTransition.patch(
+                TransitionUtil.restrict(execTransition, mappedBaselineOptions), env.getListener());
+        return BaselineOptionsValue.create(toOptions);
+      } else {
+        return BaselineOptionsValue.create(mappedBaselineOptions);
+      }
+    } catch (OptionsParsingException e) {
+      throw new BaselineOptionsFunctionException(e);
+    }
+  }
+
+  private static final class BaselineOptionsFunctionException extends SkyFunctionException {
+    BaselineOptionsFunctionException(Exception e) {
+      super(e, Transience.PERSISTENT);
+    }
+  }
+}

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

@@ -23,13 +23,13 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
-import com.google.devtools.build.lib.analysis.PlatformOptions;
 import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.CoreOptions;
 import com.google.devtools.build.lib.analysis.config.FragmentFactory;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
 import com.google.devtools.build.lib.analysis.config.OptionInfo;
+import com.google.devtools.build.lib.analysis.config.transitions.BaselineOptionsValue;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.packages.RuleClassProvider;
@@ -41,7 +41,6 @@
 import com.google.devtools.build.skyframe.SkyValue;
 import com.google.devtools.common.options.OptionDefinition;
 import com.google.devtools.common.options.OptionMetadataTag;
-import com.google.devtools.common.options.OptionsParsingException;
 import java.util.Map;
 import java.util.TreeMap;
 import javax.annotation.Nullable;
@@ -81,35 +80,25 @@ public SkyValue compute(SkyKey skyKey, Environment env)
     BuildConfigurationKey key = (BuildConfigurationKey) skyKey.argument();
 
     BuildOptions targetOptions = key.getOptions();
+    CoreOptions coreOptions = targetOptions.get(CoreOptions.class);
 
     String transitionDirectoryNameFragment;
     if (targetOptions.hasNoConfig()) {
       transitionDirectoryNameFragment = "noconfig"; // See NoConfigTransition.
-    } else if (targetOptions
-        .get(CoreOptions.class)
-        .outputDirectoryNamingScheme
-        .equals(CoreOptions.OutputDirectoryNamingScheme.DIFF_AGAINST_BASELINE)) {
-      // Herein lies a hack to apply platform mappings to the baseline options.
-      // TODO(blaze-configurability-team): this should become unnecessary once --platforms is marked
-      //   as EXPLICIT_IN_OUTPUT_PATH
-      PlatformMappingValue platformMappingValue =
-          (PlatformMappingValue)
-              env.getValue(
-                  PlatformMappingValue.Key.create(
-                      targetOptions.get(PlatformOptions.class).platformMappings));
-      if (platformMappingValue == null) {
+    } else if (coreOptions.useBaselineForOutputDirectoryNamingScheme()) {
+      boolean applyExecTransitionToBaseline =
+          coreOptions.outputDirectoryNamingScheme.equals(
+                  CoreOptions.OutputDirectoryNamingScheme.DIFF_AGAINST_DYNAMIC_BASELINE)
+              && coreOptions.isExec;
+      var baselineOptionsValue =
+          (BaselineOptionsValue)
+              env.getValue(BaselineOptionsValue.key(applyExecTransitionToBaseline));
+      if (baselineOptionsValue == null) {
         return null;
       }
-      BuildOptions baselineOptions = PrecomputedValue.BASELINE_CONFIGURATION.get(env);
-      try {
-        BuildOptions mappedBaselineOptions =
-            BuildConfigurationKey.withPlatformMapping(platformMappingValue, baselineOptions)
-                .getOptions();
-        transitionDirectoryNameFragment =
-            computeNameFragmentWithDiff(targetOptions, mappedBaselineOptions);
-      } catch (OptionsParsingException e) {
-        throw new BuildConfigurationFunctionException(e);
-      }
+
+      transitionDirectoryNameFragment =
+          computeNameFragmentWithDiff(targetOptions, baselineOptionsValue.toOptions());
     } else {
       transitionDirectoryNameFragment =
           computeNameFragmentWithAffectedByStarlarkTransition(targetOptions);

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

@@ -94,6 +94,8 @@ public final class SkyFunctions {
   static final SkyFunctionName TEST_COMPLETION = SkyFunctionName.createHermetic("TEST_COMPLETION");
   public static final SkyFunctionName BUILD_CONFIGURATION =
       SkyFunctionName.createHermetic("BUILD_CONFIGURATION");
+  public static final SkyFunctionName BASELINE_OPTIONS =
+      SkyFunctionName.createHermetic("BASELINE_OPTIONS");
   public static final SkyFunctionName STARLARK_BUILD_SETTINGS_DETAILS =
       SkyFunctionName.createHermetic("STARLARK_BUILD_SETTINGS_DETAILS");
   // Action execution can be nondeterministic, so semi-hermetic.

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

@@ -665,6 +665,7 @@ private ImmutableMap<SkyFunctionName, SkyFunction> skyFunctions() {
     map.put(
         SkyFunctions.BUILD_CONFIGURATION,
         new BuildConfigurationFunction(directories, ruleClassProvider));
+    map.put(SkyFunctions.BASELINE_OPTIONS, new BaselineOptionsFunction());
     map.put(
         SkyFunctions.STARLARK_BUILD_SETTINGS_DETAILS, new StarlarkBuildSettingsDetailsFunction());
     map.put(SkyFunctions.WORKSPACE_NAME, new WorkspaceNameFunction());