Intern strings for python zip runfiles

src/main/java/com/google/devtools/build/lib/actions/CommandLineItem.java

@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.actions;
 
+import java.util.UUID;
 import java.util.function.Consumer;
 
 /** An interface for an object that customizes how it is expanded into a command line. */
@@ -57,6 +58,22 @@ abstract class ParametrizedMapFn<T> implements MapFn<T> {
     public abstract int maxInstancesAllowed();
   }
 
+  /**
+   * Map function which may have multiple instances, where each instance differs only by a set of
+   * defined parameters.
+   *
+   * <p>Each instance needs to return a fingerprint of parameters it depends on.</p>
+   */
+  interface DefinedParamsCacheMapFn<T> extends MapFn<T> {
+    // Returns UUID common to all instances of a given implementation of DefinedParamsCacheMapFn.
+    UUID getUUID();
+
+    // Returns bytes representing parameters of this instance.
+    // Note, it could be to replace this method to addToFingerprint, but this need to be done
+    // carefully to avoid cyclic dependencies.
+    byte[] getParamsBytes();
+  }
+
   /** Expands the object into the command line as a string. */
   String expandToCommandLine();
 

src/main/java/com/google/devtools/build/lib/bazel/rules/python/BUILD

@@ -22,6 +22,7 @@ java_library(
     deps = [
         "//src/main/java/com/google/devtools/build/lib/actions",
         "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
+        "//src/main/java/com/google/devtools/build/lib/actions:commandline_item",
         "//src/main/java/com/google/devtools/build/lib/analysis:actions/custom_command_line",
         "//src/main/java/com/google/devtools/build/lib/analysis:actions/launcher_file_write_action",
         "//src/main/java/com/google/devtools/build/lib/analysis:actions/substitution",
@@ -45,6 +46,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/packages",
         "//src/main/java/com/google/devtools/build/lib/rules/cpp",
         "//src/main/java/com/google/devtools/build/lib/rules/python",
+        "//src/main/java/com/google/devtools/build/lib/util",
         "//src/main/java/com/google/devtools/build/lib/util:os",
         "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
         "//src/main/java/com/google/devtools/common/options",

src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java

@@ -20,6 +20,7 @@
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.CommandLineItem;
 import com.google.devtools.build.lib.actions.ParamFileInfo;
 import com.google.devtools.build.lib.actions.ParameterFile;
 import com.google.devtools.build.lib.analysis.AnalysisUtils;
@@ -32,6 +33,7 @@
 import com.google.devtools.build.lib.analysis.ShToolchain;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
+import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.VectorArg;
 import com.google.devtools.build.lib.analysis.actions.LauncherFileWriteAction;
 import com.google.devtools.build.lib.analysis.actions.LauncherFileWriteAction.LaunchInfo;
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
@@ -48,12 +50,15 @@
 import com.google.devtools.build.lib.rules.python.PythonSemantics;
 import com.google.devtools.build.lib.rules.python.PythonUtils;
 import com.google.devtools.build.lib.rules.python.PythonVersion;
+import com.google.devtools.build.lib.util.Fingerprint;
 import com.google.devtools.build.lib.util.OS;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import java.util.UUID;
+import java.util.function.Consumer;
 import java.util.function.Predicate;
 import javax.annotation.Nullable;
 
@@ -342,6 +347,52 @@ private static String getZipRunfilesPath(
     return getZipRunfilesPath(PathFragment.create(path), workspaceName, legacyExternalRunfiles);
   }
 
+  private static class ZipPathArgMapper implements CommandLineItem.DefinedParamsCacheMapFn<Artifact> {
+    private static final UUID MAPPER_UUID = UUID.fromString("a27af967-a391-4214-9e9d-14fd548ce063");
+
+    private Artifact executable;
+    private Artifact zipFile;
+    private PathFragment workspaceName;
+    private boolean legacyExternalRunfiles;
+
+    ZipPathArgMapper(
+        Artifact executable,
+        Artifact zipFile,
+        PathFragment workspaceName,
+        boolean legacyExternalRunfiles
+    ) {
+      this.executable = executable;
+      this.zipFile = zipFile;
+      this.workspaceName = workspaceName;
+      this.legacyExternalRunfiles = legacyExternalRunfiles;
+    }
+
+    @Override
+    public void expandToCommandLine(Artifact artifact, Consumer<String> args) {
+      if (!artifact.equals(executable) && !artifact.equals(zipFile)) {
+        args.accept(
+            getZipRunfilesPath(artifact.getRunfilesPath(), workspaceName, legacyExternalRunfiles)
+                + "="
+                + artifact.getExecPathString());
+      }
+    }
+
+    @Override
+    public UUID getUUID() {
+      return MAPPER_UUID;
+    }
+
+    @Override
+    public byte[] getParamsBytes() {
+      Fingerprint fp = new Fingerprint();
+      fp.addString(executable.getExecPathString());
+      fp.addString(zipFile.getExecPathString());
+      fp.addPath(workspaceName);
+      fp.addBoolean(legacyExternalRunfiles);
+      return fp.digestAndReset();
+    }
+  }
+
   private static void createPythonZipAction(
       RuleContext ruleContext,
       Artifact executable,
@@ -367,12 +418,12 @@ private static void createPythonZipAction(
 
     // Read each runfile from execute path, add them into zip file at the right runfiles path.
     // Filter the executable file, cause we are building it.
+    argv.addAll(VectorArg.of(runfilesSupport.getRunfilesArtifacts()).mapped(
+        new ZipPathArgMapper(executable, zipFile, workspaceName, legacyExternalRunfiles)));
+
+    // TODO: Consider adding all files to avoid expanding nested sets.
     for (Artifact artifact : runfilesSupport.getRunfilesArtifacts().toList()) {
       if (!artifact.equals(executable) && !artifact.equals(zipFile)) {
-        argv.addDynamicString(
-            getZipRunfilesPath(artifact.getRunfilesPath(), workspaceName, legacyExternalRunfiles)
-                + "="
-                + artifact.getExecPathString());
         inputsBuilder.add(artifact);
       }
     }

src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSetFingerprintCache.java

@@ -49,7 +49,13 @@ public <T> void addNestedSetToFingerprint(
       fingerprint.addInt(EMPTY_SET_DIGEST);
       return;
     }
-    DigestMap digestMap = mapFnToDigestMap.computeIfAbsent(mapFn, this::newDigestMap);
+    DigestMap digestMap;
+    if (mapFn instanceof CommandLineItem.DefinedParamsCacheMapFn) {
+      fingerprint.addUUID(((CommandLineItem.DefinedParamsCacheMapFn) mapFn).getUUID());
+      fingerprint.addBytes(((CommandLineItem.DefinedParamsCacheMapFn) mapFn).getParamsBytes());
+      mapFn = CommandLineItem.MapFn.DEFAULT;
+    }
+    digestMap = mapFnToDigestMap.computeIfAbsent(mapFn, this::newDigestMap);
     fingerprint.addInt(nestedSet.getOrder().ordinal());
     Object children = nestedSet.getChildren();
     addToFingerprint(mapFn, fingerprint, digestMap, children);