Switch MemProf profile handling to use the same zipper as FDO

Removes use of a defaultZipper for MemProf profiles that was added in 617a3bd. PiperOrigin-RevId: 579332271 Change-Id: I7784ffa9d5e3c13d4cd025eb175a0dde17c8eaa0
bazelbuild · Nov 3, 2023 · 19c03f7 · 19c03f7
1 parent 718f400
commit 19c03f7
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 26 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainAttributesProvider.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainAttributesProvider.java
@@ -85,7 +85,6 @@ public class CcToolchainAttributesProvider extends NativeInfo implements HasCcTo
   private final TransitiveInfoCollection moduleMap;
   private final Artifact moduleMapArtifact;
   private final Artifact zipper;
-  private final Artifact defaultZipper;
   private final String purposePrefix;
   private final String runtimeSolibDirBase;
   private final LicensesProvider licensesProvider;
@@ -175,7 +174,6 @@ public CcToolchainAttributesProvider(
     this.moduleMap = ruleContext.getPrerequisite("module_map");
     this.moduleMapArtifact = ruleContext.getPrerequisiteArtifact("module_map");
     this.zipper = ruleContext.getPrerequisiteArtifact(":zipper");
-    this.defaultZipper = ruleContext.getPrerequisiteArtifact(":default_zipper");
     this.purposePrefix = Actions.escapeLabel(ruleContext.getLabel()) + "_";
     this.runtimeSolibDirBase = "_solib_" + "_" + Actions.escapeLabel(ruleContext.getLabel());
     this.staticRuntimeLib = ruleContext.getPrerequisite("static_runtime_lib");
@@ -454,16 +452,10 @@ public FdoProfileProvider getXFdoProfileProvider() {
     return xfdoProfileProvider;
   }
 
-  /* Get the FDO-specific zipper. */
   public Artifact getZipper() {
     return zipper;
   }
 
-  /* Get the non FDO-specific zipper. */
-  public Artifact getDefaultZipper() {
-    return defaultZipper;
-  }
-
   public NestedSet<Artifact> getFullInputsForLink() {
     return fullInputsForLink;
   }

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
@@ -305,21 +305,15 @@ public Label getRuleProvidingCcToolchainProvider() {
   @Nullable
   @StarlarkConfigurationField(name = "zipper", doc = "The zipper label for FDO.")
   public Label getFdoZipper() {
-    if (getFdoOptimizeLabel() != null || getFdoProfileLabel() != null || getFdoPath() != null) {
+    if (getFdoOptimizeLabel() != null
+        || getFdoProfileLabel() != null
+        || getFdoPath() != null
+        || getMemProfProfileLabel() != null) {
       return Label.parseCanonicalUnchecked(BAZEL_TOOLS_REPO + "//tools/zip:unzip_fdo");
     }
     return null;
   }
 
-  @Nullable
-  @StarlarkConfigurationField(name = "default_zipper", doc = "The default zipper label for FDO.")
-  public Label getDefaultFdoZipper() {
-    if (getMemProfProfileLabel() != null) {
-      return Label.parseCanonicalUnchecked(BAZEL_TOOLS_REPO + "//tools/zip:zipper");
-    }
-    return null;
-  }
-
   /** Returns the configured current compilation mode. */
   public CompilationMode getCompilationMode() {
     return compilationMode;

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoHelper.java
@@ -488,7 +488,7 @@ private static Artifact getMemProfProfileArtifact(
     }
 
     // Get the zipper binary for unzipping the profile.
-    Artifact zipperBinaryArtifact = attributes.getDefaultZipper();
+    Artifact zipperBinaryArtifact = attributes.getZipper();
     if (zipperBinaryArtifact == null) {
       if (CppHelper.useToolchainResolution(ruleContext)) {
         ruleContext.ruleError(
@@ -513,8 +513,17 @@ private static Artifact getMemProfProfileArtifact(
         "Symlinking MemProf ZIP Profile " + memprofProfile.getBasename());
 
     CustomCommandLine.Builder argv = new CustomCommandLine.Builder();
-    argv.addExecPath("xf", zipProfileArtifact)
-        .add("-d", profileArtifact.getExecPath().getParentDirectory().getSafePathString());
+    // We invoke different binaries depending on whether the unzip_fdo tool
+    // is available. When it isn't, unzip_fdo is aliased to the generic
+    // zipper tool, which takes different command-line arguments.
+    if (zipperBinaryArtifact.getExecPathString().endsWith("unzip_fdo")) {
+      argv.addExecPath("--profile_zip", zipProfileArtifact)
+          .add("--memprof")
+          .add("--output_file", profileArtifact.getExecPath().getSafePathString());
+    } else {
+      argv.addExecPath("xf", zipProfileArtifact)
+          .add("-d", profileArtifact.getExecPath().getParentDirectory().getSafePathString());
+    }
     // Unzip the profile.
     ruleContext.registerAction(
         new SpawnAction.Builder()

diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_toolchain_attrs.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_toolchain_attrs.bzl
@@ -129,11 +129,6 @@ cc_toolchain_attrs_exec = {
         allow_single_file = True,
         cfg = "exec",
     ),
-    "_default_zipper": attr.label(
-        default = configuration_field(fragment = "cpp", name = "default_zipper"),
-        allow_single_file = True,
-        cfg = "exec",
-    ),
     "_target_libc_top": attr.label(
         default = configuration_field(fragment = "cpp", name = "target_libc_top_DO_NOT_USE_ONLY_FOR_CC_TOOLCHAIN"),
         cfg = "target",