Push makeExecutable down into AbstractFileWriteAction subclasses

Getting rid of the single boolean field on `AbstractFileWriteAction` reduces padding on each subclass instance and in particular frees up a 4-byte field on `CppModuleMapAction`.

Also use a lambda to define `newDeterministicWriter` if possible for improved readability.

This prepares for future changes that will add fields to `CppModuleMapAction` to support path mapping.

Work towards bazelbuild#6526

Closes bazelbuild#22609.

PiperOrigin-RevId: 643340715
Change-Id: Id3af26049098e6dfa731f0e7a1be6709bea0d9f2

src/main/java/com/google/devtools/build/lib/analysis/RepoMappingManifestAction.java

@@ -105,8 +105,7 @@ public RepoMappingManifestAction(
       NestedSet<SymlinkEntry> runfilesSymlinks,
       NestedSet<SymlinkEntry> runfilesRootSymlinks,
       String workspaceName) {
-    super(
-        owner, NestedSetBuilder.emptySet(Order.STABLE_ORDER), output, /* makeExecutable= */ false);
+    super(owner, NestedSetBuilder.emptySet(Order.STABLE_ORDER), output);
     this.transitivePackages = transitivePackages;
     this.runfilesArtifacts = runfilesArtifacts;
     this.hasRunfilesSymlinks = !runfilesSymlinks.isEmpty();

src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java

@@ -143,7 +143,7 @@ public SourceManifestAction(
       @Nullable Artifact repoMappingManifest,
       boolean remotableSourceManifestActions) {
     // The real set of inputs is computed in #getInputs().
-    super(owner, NestedSetBuilder.emptySet(Order.STABLE_ORDER), primaryOutput, false);
+    super(owner, NestedSetBuilder.emptySet(Order.STABLE_ORDER), primaryOutput);
     this.manifestWriter = manifestWriter;
     this.runfiles = runfiles;
     this.repoMappingManifest = repoMappingManifest;

src/main/java/com/google/devtools/build/lib/analysis/actions/AbstractFileWriteAction.java

@@ -33,26 +33,20 @@
  * Abstract Action to write to a file.
  */
 public abstract class AbstractFileWriteAction extends AbstractAction {
-
-  protected final boolean makeExecutable;
-
   /**
    * Creates a new AbstractFileWriteAction instance.
    *
    * @param owner the action owner.
    * @param inputs the Artifacts that this Action depends on
    * @param output the Artifact that will be created by executing this Action.
-   * @param makeExecutable iff true will change the output file to be executable.
    */
-  public AbstractFileWriteAction(
-      ActionOwner owner, NestedSet<Artifact> inputs, Artifact output, boolean makeExecutable) {
+  public AbstractFileWriteAction(ActionOwner owner, NestedSet<Artifact> inputs, Artifact output) {
     // There is only one output, and it is primary.
     super(owner, inputs, ImmutableSet.of(output));
-    this.makeExecutable = makeExecutable;
   }
 
   public boolean makeExecutable() {
-    return makeExecutable;
+    return false;
   }
 
   @Override
@@ -64,7 +58,7 @@ public final ActionResult execute(ActionExecutionContext actionExecutionContext)
           actionExecutionContext.getContext(FileWriteActionContext.class);
       ImmutableList<SpawnResult> result =
           context.writeOutputToFile(
-              this, actionExecutionContext, deterministicWriter, makeExecutable, isRemotable());
+              this, actionExecutionContext, deterministicWriter, makeExecutable(), isRemotable());
       afterWrite(actionExecutionContext);
       return ActionResult.create(result);
     } catch (ExecException e) {
@@ -95,7 +89,7 @@ public String getMnemonic() {
 
   @Override
   protected String getRawProgressMessage() {
-    return (makeExecutable ? "Writing script " : "Writing file ")
+    return (makeExecutable() ? "Writing script " : "Writing file ")
         + Iterables.getOnlyElement(getOutputs()).prettyPrint();
   }
 

src/main/java/com/google/devtools/build/lib/analysis/actions/BinaryFileWriteAction.java

@@ -29,7 +29,6 @@
 import com.google.devtools.build.lib.util.Fingerprint;
 import java.io.IOException;
 import java.io.InputStream;
-import java.io.OutputStream;
 import javax.annotation.Nullable;
 
 /**
@@ -41,6 +40,7 @@ public final class BinaryFileWriteAction extends AbstractFileWriteAction {
   private static final String GUID = "eeee07fe-4b40-11e4-82d6-eba0b4f713e2";
 
   private final ByteSource source;
+  private final boolean makeExecutable;
 
   /**
    * Creates a new BinaryFileWriteAction instance without inputs.
@@ -52,8 +52,14 @@ public final class BinaryFileWriteAction extends AbstractFileWriteAction {
    */
   public BinaryFileWriteAction(
       ActionOwner owner, Artifact output, ByteSource source, boolean makeExecutable) {
-    super(owner, /*inputs=*/ NestedSetBuilder.emptySet(Order.STABLE_ORDER), output, makeExecutable);
+    super(owner, /* inputs= */ NestedSetBuilder.emptySet(Order.STABLE_ORDER), output);
     this.source = Preconditions.checkNotNull(source);
+    this.makeExecutable = makeExecutable;
+  }
+
+  @Override
+  public boolean makeExecutable() {
+    return makeExecutable;
   }
 
   @VisibleForTesting
@@ -63,14 +69,11 @@ public ByteSource getSource() {
 
   @Override
   public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) {
-    return new DeterministicWriter() {
-      @Override
-      public void writeOutputFile(OutputStream out) throws IOException {
-        try (InputStream in = source.openStream()) {
-          ByteStreams.copy(in, out);
-        }
-        out.flush();
+    return out -> {
+      try (InputStream in = source.openStream()) {
+        ByteStreams.copy(in, out);
       }
+      out.flush();
     };
   }
 
@@ -80,7 +83,7 @@ protected void computeKey(
       @Nullable ArtifactExpander artifactExpander,
       Fingerprint fp) {
     fp.addString(GUID);
-    fp.addString(String.valueOf(makeExecutable));
+    fp.addBoolean(makeExecutable());
 
     try (InputStream in = source.openStream()) {
       byte[] buffer = new byte[512];

src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteAction.java

@@ -69,6 +69,8 @@ public abstract class FileWriteAction extends AbstractFileWriteAction
   /** Minimum length (in chars) for content to be eligible for compression. */
   private static final int COMPRESS_CHARS_THRESHOLD = 256;
 
+  private final boolean makeExecutable;
+
   /**
    * Creates a FileWriteAction to write contents to the resulting artifact fileName in the genfiles
    * root underneath the package path.
@@ -169,14 +171,20 @@ private FileWriteAction(
       NestedSet<Artifact> inputs,
       Artifact primaryOutput,
       boolean makeExecutable) {
-    super(owner, inputs, primaryOutput, makeExecutable);
+    super(owner, inputs, primaryOutput);
+    this.makeExecutable = makeExecutable;
   }
 
   @Override
   public final String getFileContents(@Nullable EventHandler eventHandler) {
     return getFileContents();
   }
 
+  @Override
+  public boolean makeExecutable() {
+    return makeExecutable;
+  }
+
   /**
    * Returns the string contents to be written.
    *
@@ -225,7 +233,7 @@ protected void computeKey(
         ActionKeyContext actionKeyContext,
         @Nullable ArtifactExpander artifactExpander,
         Fingerprint fp) {
-      fp.addString(GUID).addBoolean(makeExecutable).addString(getFileContents());
+      fp.addString(GUID).addBoolean(makeExecutable()).addString(getFileContents());
     }
   }
 
@@ -305,7 +313,7 @@ protected void computeKey(
         ActionKeyContext actionKeyContext,
         @Nullable ArtifactExpander artifactExpander,
         Fingerprint fp) {
-      fp.addString(GUID).addBoolean(makeExecutable).addBytes(compressedBytes);
+      fp.addString(GUID).addBoolean(makeExecutable()).addBytes(compressedBytes);
     }
   }
 }

src/main/java/com/google/devtools/build/lib/analysis/actions/LauncherFileWriteAction.java

@@ -62,13 +62,17 @@ private LauncherFileWriteAction(
     super(
         ruleContext.getActionOwner(),
         NestedSetBuilder.create(Order.STABLE_ORDER, Preconditions.checkNotNull(launcher)),
-        output,
-        /* makeExecutable= */ true);
+        output);
     this.launcher = launcher; // already null-checked in the superclass c'tor
     this.launchInfo = Preconditions.checkNotNull(launchInfo);
     this.isExecutedOnWindows = ruleContext.isExecutedOnWindows();
   }
 
+  @Override
+  public boolean makeExecutable() {
+    return true;
+  }
+
   @Override
   public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) {
     // TODO(tjgq): Move this check into createAndRegister.

src/main/java/com/google/devtools/build/lib/analysis/actions/LazyWriteNestedSetOfTupleAction.java

@@ -26,8 +26,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.util.Fingerprint;
-import java.io.IOException;
-import java.io.OutputStream;
 import javax.annotation.Nullable;
 import net.starlark.java.eval.Tuple;
 
@@ -44,20 +42,14 @@ public final class LazyWriteNestedSetOfTupleAction extends AbstractFileWriteActi
 
   public LazyWriteNestedSetOfTupleAction(
       ActionOwner owner, Artifact output, NestedSet<Tuple> tuplesToWrite, String delimiter) {
-    super(
-        owner, NestedSetBuilder.emptySet(Order.STABLE_ORDER), output, /* makeExecutable= */ false);
+    super(owner, NestedSetBuilder.emptySet(Order.STABLE_ORDER), output);
     this.tuplesToWrite = tuplesToWrite;
     this.delimiter = delimiter;
   }
 
   @Override
   public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) {
-    return new DeterministicWriter() {
-      @Override
-      public void writeOutputFile(OutputStream out) throws IOException {
-        out.write(getContents(delimiter).getBytes(UTF_8));
-      }
-    };
+    return out -> out.write(getContents(delimiter).getBytes(UTF_8));
   }
 
   /** Computes the Action key for this action by computing the fingerprint for the file contents. */

src/main/java/com/google/devtools/build/lib/analysis/actions/LazyWritePathsFileAction.java

@@ -27,8 +27,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.util.Fingerprint;
-import java.io.IOException;
-import java.io.OutputStream;
 import java.util.function.Function;
 import javax.annotation.Nullable;
 
@@ -64,7 +62,7 @@ public LazyWritePathsFileAction(
       Function<Artifact, String> converter) {
     // We don't need to pass the given files as explicit inputs to this action; we don't care about
     // them, we only need their names, which we already know.
-    super(owner, NestedSetBuilder.emptySet(Order.STABLE_ORDER), output, false);
+    super(owner, NestedSetBuilder.emptySet(Order.STABLE_ORDER), output);
     this.files = files;
     this.includeDerivedArtifacts = includeDerivedArtifacts;
     this.filesToIgnore = filesToIgnore;
@@ -73,12 +71,7 @@ public LazyWritePathsFileAction(
 
   @Override
   public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) {
-    return new DeterministicWriter() {
-      @Override
-      public void writeOutputFile(OutputStream out) throws IOException {
-        out.write(getContents().toString().getBytes(UTF_8));
-      }
-    };
+    return out -> out.write(getContents().getBytes(UTF_8));
   }
 
   /** Computes the Action key for this action by computing the fingerprint for the file contents. */

src/main/java/com/google/devtools/build/lib/analysis/actions/ParameterFileWriteAction.java

@@ -86,7 +86,7 @@ public ParameterFileWriteAction(
       Artifact output,
       CommandLine commandLine,
       ParameterFileType type) {
-    super(owner, inputs, output, false);
+    super(owner, inputs, output);
     this.commandLine = commandLine;
     this.type = type;
     this.hasInputArtifactToExpand = !inputs.isEmpty();
@@ -174,7 +174,6 @@ protected void computeKey(
       Fingerprint fp)
       throws CommandLineExpansionException, InterruptedException {
     fp.addString(GUID);
-    fp.addString(String.valueOf(makeExecutable));
     fp.addString(type.toString());
     commandLine.addToFingerprint(actionKeyContext, artifactExpander, fp);
   }
@@ -184,8 +183,6 @@ public String describeKey() {
     StringBuilder message = new StringBuilder();
     message.append("GUID: ");
     message.append(GUID);
-    message.append("\nExecutable: ");
-    message.append(makeExecutable);
     message.append("\nParam File Type: ");
     message.append(type);
     message.append("\nContent digest (approximate): ");

src/main/java/com/google/devtools/build/lib/analysis/extra/ExtraActionInfoFileWriteAction.java

@@ -53,8 +53,7 @@ public final class ExtraActionInfoFileWriteAction extends AbstractFileWriteActio
         shadowedAction.discoversInputs()
             ? NestedSetBuilder.<Artifact>stableOrder().addAll(shadowedAction.getOutputs()).build()
             : NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER),
-        primaryOutput,
-        /*makeExecutable=*/ false);
+        primaryOutput);
 
     this.shadowedAction = Preconditions.checkNotNull(shadowedAction, primaryOutput);
   }

src/main/java/com/google/devtools/build/lib/analysis/test/BaselineCoverageAction.java

@@ -33,8 +33,6 @@
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.util.Fingerprint;
 import com.google.devtools.build.lib.vfs.PathFragment;
-import java.io.IOException;
-import java.io.OutputStream;
 import java.io.PrintWriter;
 import javax.annotation.Nullable;
 
@@ -47,7 +45,7 @@ public final class BaselineCoverageAction extends AbstractFileWriteAction
 
   private BaselineCoverageAction(
       ActionOwner owner, NestedSet<Artifact> instrumentedFiles, Artifact primaryOutput) {
-    super(owner, NestedSetBuilder.emptySet(Order.STABLE_ORDER), primaryOutput, false);
+    super(owner, NestedSetBuilder.emptySet(Order.STABLE_ORDER), primaryOutput);
     this.instrumentedFiles = instrumentedFiles;
   }
 
@@ -68,16 +66,13 @@ public void computeKey(
 
   @Override
   public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) {
-    return new DeterministicWriter() {
-      @Override
-      public void writeOutputFile(OutputStream out) throws IOException {
-        PrintWriter writer = new PrintWriter(out);
-        for (Artifact file : instrumentedFiles.toList()) {
-          writer.write("SF:" + file.getExecPathString() + "\n");
-          writer.write("end_of_record\n");
-        }
-        writer.flush();
+    return out -> {
+      PrintWriter writer = new PrintWriter(out);
+      for (Artifact file : instrumentedFiles.toList()) {
+        writer.write("SF:" + file.getExecPathString() + "\n");
+        writer.write("end_of_record\n");
       }
+      writer.flush();
     };
   }
 

src/main/java/com/google/devtools/build/lib/analysis/test/InstrumentedFileManifestAction.java

@@ -30,8 +30,6 @@
 import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.util.Fingerprint;
-import java.io.IOException;
-import java.io.OutputStream;
 import java.io.OutputStreamWriter;
 import java.io.Writer;
 import java.util.Arrays;
@@ -48,28 +46,21 @@ final class InstrumentedFileManifestAction extends AbstractFileWriteAction {
 
   @VisibleForTesting
   InstrumentedFileManifestAction(ActionOwner owner, NestedSet<Artifact> files, Artifact output) {
-    super(
-        owner,
-        /*inputs=*/ NestedSetBuilder.emptySet(Order.STABLE_ORDER),
-        output,
-        /*makeExecutable=*/ false);
+    super(owner, /* inputs= */ NestedSetBuilder.emptySet(Order.STABLE_ORDER), output);
     this.files = files;
   }
 
   @Override
   public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) {
-    return new DeterministicWriter() {
-      @Override
-      public void writeOutputFile(OutputStream out) throws IOException {
-        // Sort the exec paths before writing them out.
-        String[] fileNames =
-            files.toList().stream().map(Artifact::getExecPathString).toArray(String[]::new);
-        Arrays.sort(fileNames);
-        try (Writer writer = new OutputStreamWriter(out, ISO_8859_1)) {
-          for (String name : fileNames) {
-            writer.write(name);
-            writer.write('\n');
-          }
+    return out -> {
+      // Sort the exec paths before writing them out.
+      String[] fileNames =
+          files.toList().stream().map(Artifact::getExecPathString).toArray(String[]::new);
+      Arrays.sort(fileNames);
+      try (Writer writer = new OutputStreamWriter(out, ISO_8859_1)) {
+        for (String name : fileNames) {
+          writer.write(name);
+          writer.write('\n');
         }
       }
     };

src/main/java/com/google/devtools/build/lib/rules/android/AndroidDeployInfoAction.java

@@ -67,11 +67,7 @@ private static NestedSet<Artifact> makeInputs(
       Artifact mergedManifest,
       ImmutableList<Artifact> additionalMergedManifests,
       ImmutableList<Artifact> apksToDeploy) {
-    super(
-        owner,
-        makeInputs(mergedManifest, additionalMergedManifests, apksToDeploy),
-        outputFile,
-        false);
+    super(owner, makeInputs(mergedManifest, additionalMergedManifests, apksToDeploy), outputFile);
     this.mergedManifest = mergedManifest;
     this.additionalMergedManifests = additionalMergedManifests;
     this.apksToDeploy = apksToDeploy;