Set up only one link action for JavaBinary launcher with fission

Change getLauncher to return both a stripped and unstripped launcher binary
artifact under fission, instead of invoking getLauncher twice. This was
setting up two identical link actions that required later work to filter
out the redundant action in filterSharedActionsAndThrowActionConflict.
This becomes extremely inefficient under ThinLTO, where each launcher link
is actually 1 LTO indexing action, N LTO Backend actions, and 1 native link
action.

RELNOTES: None
PiperOrigin-RevId: 194781580

3 files changed, 40 insertions, 22 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java
index ecb65b4b7a..bcc9ea817b 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java
@@ -68,6 +68,7 @@ import com.google.devtools.build.lib.rules.java.proto.GeneratedExtensionRegistry
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.OS;
+import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.util.ShellEscaper;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -773,10 +774,17 @@ public class BazelJavaSemantics implements JavaSemantics {
   }
 
   @Override
-  public Artifact getLauncher(RuleContext ruleContext, JavaCommon common,
-      DeployArchiveBuilder deployArchiveBuilder, Runfiles.Builder runfilesBuilder,
-      List<String> jvmFlags, JavaTargetAttributes.Builder attributesBuilder, boolean shouldStrip) {
-    return JavaHelper.launcherArtifactForTarget(this, ruleContext);
+  public Pair<Artifact, Artifact> getLauncher(
+      RuleContext ruleContext,
+      JavaCommon common,
+      DeployArchiveBuilder deployArchiveBuilder,
+      DeployArchiveBuilder unstrippedDeployArchiveBuilder,
+      Runfiles.Builder runfilesBuilder,
+      List<String> jvmFlags,
+      JavaTargetAttributes.Builder attributesBuilder,
+      boolean shouldStrip) {
+    Artifact launcher = JavaHelper.launcherArtifactForTarget(this, ruleContext);
+    return new Pair<>(launcher, launcher);
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java
index f50bc03944..aa6de48b3a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java
@@ -50,6 +50,7 @@ import com.google.devtools.build.lib.rules.java.ProguardHelper.ProguardOutput;
 import com.google.devtools.build.lib.rules.java.proto.GeneratedExtensionRegistryProvider;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.OS;
+import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -140,23 +141,22 @@ public class JavaBinary implements RuleConfiguredTargetFactory {
     boolean stripAsDefault =
         CppHelper.useFission(cppConfiguration, ccToolchain)
             && cppConfiguration.getCompilationMode() == CompilationMode.OPT;
-    Artifact launcher = semantics.getLauncher(ruleContext, common, deployArchiveBuilder,
-        runfilesBuilder, jvmFlags, attributesBuilder, stripAsDefault);
-
     DeployArchiveBuilder unstrippedDeployArchiveBuilder = null;
-    Artifact unstrippedLauncher = null;
     if (stripAsDefault) {
       unstrippedDeployArchiveBuilder = new DeployArchiveBuilder(semantics, ruleContext);
-      unstrippedLauncher =
-          semantics.getLauncher(
-              ruleContext,
-              common,
-              unstrippedDeployArchiveBuilder,
-              runfilesBuilder,
-              jvmFlags,
-              attributesBuilder,
-              /* shouldStrip= */ false);
     }
+    Pair<Artifact, Artifact> launcherAndUnstrippedLauncher =
+        semantics.getLauncher(
+            ruleContext,
+            common,
+            deployArchiveBuilder,
+            unstrippedDeployArchiveBuilder,
+            runfilesBuilder,
+            jvmFlags,
+            attributesBuilder,
+            stripAsDefault);
+    Artifact launcher = launcherAndUnstrippedLauncher.first;
+    Artifact unstrippedLauncher = launcherAndUnstrippedLauncher.second;
 
     JavaCompilationArtifacts.Builder javaArtifactsBuilder = new JavaCompilationArtifacts.Builder();
     Artifact instrumentationMetadata =
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java
index d6b7d60527..fdf59887a7 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java
@@ -45,6 +45,7 @@ import com.google.devtools.build.lib.rules.java.JavaConfiguration.OneVersionEnfo
 import com.google.devtools.build.lib.rules.java.proto.GeneratedExtensionRegistryProvider;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.FileType;
+import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.io.File;
 import java.util.Collection;
@@ -440,16 +441,25 @@ public interface JavaSemantics {
    * @param ruleContext The rule context
    * @param common The common helper class.
    * @param deployArchiveBuilder the builder to construct the deploy archive action (mutable).
+   * @param unstrippedDeployArchiveBuilder the builder to construct the unstripped deploy archive
+   *     action (mutable).
    * @param runfilesBuilder the builder to construct the list of runfiles (mutable).
    * @param jvmFlags the list of flags to pass to the JVM when running the Java binary (mutable).
    * @param attributesBuilder the builder to construct the list of attributes of this target
-   *        (mutable).
-   * @return the launcher as an artifact
+   *     (mutable).
+   * @return the launcher and unstripped launcher as an artifact pair. If shouldStrip is false, then
+   *     they will be the same.
    * @throws InterruptedException
    */
-  Artifact getLauncher(final RuleContext ruleContext, final JavaCommon common,
-      DeployArchiveBuilder deployArchiveBuilder, Runfiles.Builder runfilesBuilder,
-      List<String> jvmFlags, JavaTargetAttributes.Builder attributesBuilder, boolean shouldStrip)
+  Pair<Artifact, Artifact> getLauncher(
+      final RuleContext ruleContext,
+      final JavaCommon common,
+      DeployArchiveBuilder deployArchiveBuilder,
+      DeployArchiveBuilder unstrippedDeployArchiveBuilder,
+      Runfiles.Builder runfilesBuilder,
+      List<String> jvmFlags,
+      JavaTargetAttributes.Builder attributesBuilder,
+      boolean shouldStrip)
       throws InterruptedException;
 
   /**