Use chaining to reduce duplicate toolchain vars

The idea is that all actions generated the same CcCompilationHelper
share a set of toolchain variables (especially those passed in as
extra variables), which significantly reduces memory consumption in
cases where Bazel generates more than one actions for a single
configured target.

AFAICT, there does not seem to be a mechanism that allows the extra
variables to vary based on the specific action generated (i.e., based on
the source or output file for that action).

Note that we can't share copts in the case where per_file_copts is set.
Ideally, we'd share copts as well, unless specifically overridden.

PiperOrigin-RevId: 259276353

src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java

@@ -41,7 +41,6 @@
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
-import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
 import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
 import com.google.devtools.build.lib.packages.RuleErrorConsumer;
@@ -87,10 +86,8 @@ public final class CcCompilationHelper {
    * Configures a compile action builder by setting up command line options and auxiliary inputs
    * according to the FDO configuration. This method does nothing If FDO is disabled.
    */
-  @ThreadSafe
-  public static void configureFdoBuildVariables(
-      ImmutableMap.Builder<String, String> variablesBuilder,
-      CppCompileActionBuilder builder,
+  private static void configureFdoBuildVariables(
+      Map<String, String> variablesBuilder,
       FeatureConfiguration featureConfiguration,
       FdoContext fdoContext,
       String fdoInstrument,
@@ -117,13 +114,10 @@ public static void configureFdoBuildVariables(
           fdoContext.getPrefetchHintsArtifact().getExecPathString());
     }
 
-    Iterable<Artifact> auxiliaryInputs = getAuxiliaryFdoInputs(fdoContext);
-    builder.addMandatoryInputs(auxiliaryInputs);
-
     FdoContext.BranchFdoProfile branchFdoProfile = fdoContext.getBranchFdoProfile();
     // Optimization phase
     if (branchFdoProfile != null) {
-      if (!Iterables.isEmpty(auxiliaryInputs)) {
+      if (!Iterables.isEmpty(getAuxiliaryFdoInputs(fdoContext))) {
         if (featureConfiguration.isEnabled(CppRuleClasses.AUTOFDO)
             || featureConfiguration.isEnabled(CppRuleClasses.XBINARYFDO)) {
           variablesBuilder.put(
@@ -255,6 +249,8 @@ public CcCompilationContext getCcCompilationContext() {
 
   private final SourceCategory sourceCategory;
   private final List<VariablesExtension> variablesExtensions = new ArrayList<>();
+  private CcToolchainVariables prebuiltParent;
+  private CcToolchainVariables prebuiltParentWithFdo;
   @Nullable private CppModuleMap cppModuleMap;
   private boolean propagateModuleMapToCompileAction = true;
 
@@ -1361,7 +1357,7 @@ private Artifact createCompileActionTemplate(
             builder,
             /* sourceLabel= */ null,
             usePic,
-            /* ccRelativeName= */ null,
+            /* needsFdoBuildVariables= */ false,
             ccCompilationContext.getCppModuleMap(),
             /* gcnoFile= */ null,
             /* isUsingFission= */ false,
@@ -1423,7 +1419,7 @@ private CcToolchainVariables setupCompileBuildVariables(
       CppCompileActionBuilder builder,
       Label sourceLabel,
       boolean usePic,
-      PathFragment ccRelativeName,
+      boolean needsFdoBuildVariables,
       CppModuleMap cppModuleMap,
       Artifact gcnoFile,
       boolean isUsingFission,
@@ -1435,51 +1431,94 @@ private CcToolchainVariables setupCompileBuildVariables(
     if (builder.getDotdFile() != null) {
       dotdFileExecPath = builder.getDotdFile().getExecPathString();
     }
-    ImmutableMap.Builder<String, String> allAdditionalBuildVariables = ImmutableMap.builder();
-    allAdditionalBuildVariables.putAll(additionalBuildVariables);
-    if (ccRelativeName != null) {
-      configureFdoBuildVariables(
-          allAdditionalBuildVariables,
-          builder,
+    if (needsFdoBuildVariables && fdoContext.hasArtifacts(cppConfiguration)) {
+      // This modifies the passed-in builder, which is a surprising side-effect, and makes it unsafe
+      // to call this method multiple times for the same builder.
+      builder.addMandatoryInputs(getAuxiliaryFdoInputs(fdoContext));
+    }
+    CcToolchainVariables parent = needsFdoBuildVariables ? prebuiltParentWithFdo : prebuiltParent;
+    // We use the prebuilt parent variables if and only if the passed in cppModuleMap is the
+    // identical to the one returned from ccCompilationContext.getCppModuleMap(): there is exactly
+    // one caller which passes in any other value (the verification module map), so this should be
+    // fine for now.
+    boolean usePrebuiltParent =
+        cppModuleMap == ccCompilationContext.getCppModuleMap()
+            // Only use the prebuilt parent if there are enough sources to make it worthwhile. The
+            // threshold was chosen by looking at a heap dump.
+            && (compilationUnitSources.size() > 1);
+    CcToolchainVariables.Builder buildVariables;
+    if (parent != null && usePrebuiltParent) {
+      // If we have a pre-built parent and we are allowed to use it, then do so.
+      buildVariables = CcToolchainVariables.builder(parent);
+    } else {
+      Map<String, String> genericAdditionalBuildVariables = new LinkedHashMap<>();
+      if (needsFdoBuildVariables) {
+        configureFdoBuildVariables(
+            genericAdditionalBuildVariables,
+            featureConfiguration,
+            fdoContext,
+            cppConfiguration.getFdoInstrument(),
+            cppConfiguration.getCSFdoInstrument(),
+            cppConfiguration);
+      }
+      buildVariables =
+          CcToolchainVariables.builder(
+              ccToolchain.getBuildVariables(configuration.getOptions(), cppConfiguration));
+      CompileBuildVariables.setupCommonVariables(
+          buildVariables,
           featureConfiguration,
-          fdoContext,
-          cppConfiguration.getFdoInstrument(),
-          cppConfiguration.getCSFdoInstrument(),
-          cppConfiguration);
+          ImmutableList.of(),
+          cppModuleMap,
+          CppHelper.getFdoBuildStamp(cppConfiguration, fdoContext, featureConfiguration),
+          variablesExtensions,
+          genericAdditionalBuildVariables,
+          ccCompilationContext.getDirectModuleMaps(),
+          ccCompilationContext.getIncludeDirs(),
+          ccCompilationContext.getQuoteIncludeDirs(),
+          ccCompilationContext.getSystemIncludeDirs(),
+          ccCompilationContext.getFrameworkIncludeDirs(),
+          ccCompilationContext.getDefines());
+
+      if (usePrebuiltParent) {
+        parent = buildVariables.build();
+        if (needsFdoBuildVariables) {
+          prebuiltParentWithFdo = parent;
+        } else {
+          prebuiltParent = parent;
+        }
+        buildVariables = CcToolchainVariables.builder(parent);
+      }
     }
-    return CompileBuildVariables.setupVariablesOrReportRuleError(
-        ruleErrorConsumer,
-        featureConfiguration,
-        ccToolchain,
-        configuration.getOptions(),
-        cppConfiguration,
+    if (usePic
+        && !featureConfiguration.isEnabled(CppRuleClasses.PIC)
+        && !featureConfiguration.isEnabled(CppRuleClasses.SUPPORTS_PIC)) {
+      ruleErrorConsumer.ruleError(CcCommon.PIC_CONFIGURATION_ERROR);
+    }
+
+    CompileBuildVariables.setupSpecificVariables(
+        buildVariables,
         toPathString(sourceFile),
         toPathString(builder.getOutputFile()),
         toPathString(gcnoFile),
-        isUsingFission,
         toPathString(dwoFile),
+        isUsingFission,
         toPathString(ltoIndexingFile),
-        ImmutableList.of(),
         getCopts(builder.getSourceFile(), sourceLabel),
-        cppModuleMap,
-        usePic,
-        builder.getTempOutputFile(),
-        CppHelper.getFdoBuildStamp(cppConfiguration, fdoContext, featureConfiguration),
+        toPathString(builder.getTempOutputFile()),
         dotdFileExecPath,
-        ImmutableList.copyOf(variablesExtensions),
-        allAdditionalBuildVariables.build(),
-        ccCompilationContext.getDirectModuleMaps(),
-        ccCompilationContext.getIncludeDirs(),
-        ccCompilationContext.getQuoteIncludeDirs(),
-        ccCompilationContext.getSystemIncludeDirs(),
-        ccCompilationContext.getFrameworkIncludeDirs(),
-        ccCompilationContext.getDefines());
+        usePic,
+        additionalBuildVariables);
+    return buildVariables.build();
   }
 
   private static String toPathString(Artifact a) {
     return a == null ? null : a.getExecPathString();
   }
 
+  private static String toPathString(PathFragment a) {
+    return a == null ? null : a.getSafePathString();
+  }
+
   /**
    * Returns a {@code CppCompileActionBuilder} with the common fields for a C++ compile action being
    * initialized.
@@ -1544,7 +1583,7 @@ private void createModuleCodegenAction(
             builder,
             sourceLabel,
             /* usePic= */ pic,
-            ccRelativeName,
+            /* needsFdoBuildVariables= */ ccRelativeName != null,
             ccCompilationContext.getCppModuleMap(),
             gcnoFile,
             generateDwo,
@@ -1598,7 +1637,7 @@ private void createHeaderAction(
             builder,
             sourceLabel,
             generatePicAction,
-            /* ccRelativeName= */ null,
+            /* needsFdoBuildVariables= */ false,
             ccCompilationContext.getCppModuleMap(),
             /* gcnoFile= */ null,
             /* isUsingFission= */ false,
@@ -1692,7 +1731,7 @@ private Collection<Artifact> createSourceAction(
                 picBuilder,
                 sourceLabel,
                 /* usePic= */ true,
-                ccRelativeName,
+                /* needsFdoBuildVariables= */ ccRelativeName != null,
                 ccCompilationContext.getCppModuleMap(),
                 gcnoFile,
                 generateDwo,
@@ -1767,7 +1806,7 @@ private Collection<Artifact> createSourceAction(
                 builder,
                 sourceLabel,
                 /* usePic= */ false,
-                ccRelativeName,
+                /* needsFdoBuildVariables= */ ccRelativeName != null,
                 cppModuleMap,
                 gcnoFile,
                 generateDwo,
@@ -1880,7 +1919,7 @@ private void createFakeSourceAction(
             builder,
             sourceLabel,
             usePic,
-            ccRelativeName,
+            /* needsFdoBuildVariables= */ ccRelativeName != null,
             ccCompilationContext.getCppModuleMap(),
             /* gcnoFile= */ null,
             /* isUsingFission= */ false,
@@ -1996,7 +2035,7 @@ private ImmutableList<Artifact> createTempsActions(
             dBuilder,
             sourceLabel,
             usePic,
-            ccRelativeName,
+            /* needsFdoBuildVariables= */ ccRelativeName != null,
             ccCompilationContext.getCppModuleMap(),
             /* gcnoFile= */ null,
             /* isUsingFission= */ false,
@@ -2022,7 +2061,7 @@ private ImmutableList<Artifact> createTempsActions(
             sdBuilder,
             sourceLabel,
             usePic,
-            ccRelativeName,
+            /* needsFdoBuildVariables= */ ccRelativeName != null,
             ccCompilationContext.getCppModuleMap(),
             /* gcnoFile= */ null,
             /* isUsingFission= */ false,