Fix memory regression from CL/160891204.

Java compile actions create unnecessary wrapper objects around a shared constant object. We can share the ActionEnviroment between these actions.

In the general spawn case there will be a lot of empty action environments. Make sure that these are shared too.

RELNOTES: None
PiperOrigin-RevId: 161389056

5 files changed, 24 insertions, 13 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionEnvironment.java b/src/main/java/com/google/devtools/build/lib/actions/ActionEnvironment.java
index ba39e801ee..d074c399de 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionEnvironment.java
@@ -42,7 +42,8 @@ public final class ActionEnvironment {
    */
   // TODO(ulfjack): Migrate all production code to use the proper action environment, and then make
   // this @VisibleForTesting or rename it to clarify.
-  public static final ActionEnvironment EMPTY = new ActionEnvironment(ImmutableMap.of());
+  public static final ActionEnvironment EMPTY =
+      new ActionEnvironment(ImmutableMap.of(), ImmutableSet.of());
 
   /**
    * Splits the given map into a map of variables with a fixed value, and a set of variables that
@@ -62,24 +63,31 @@ public final class ActionEnvironment {
         inheritedEnv.add(key);
       }
     }
-    return new ActionEnvironment(fixedEnv, inheritedEnv);
+    return create(fixedEnv, inheritedEnv);
   }
 
   private final ImmutableMap<String, String> fixedEnv;
   private final ImmutableSet<String> inheritedEnv;
 
+  private ActionEnvironment(Map<String, String> fixedEnv, Set<String> inheritedEnv) {
+    this.fixedEnv = ImmutableMap.copyOf(fixedEnv);
+    this.inheritedEnv = ImmutableSet.copyOf(inheritedEnv);
+  }
+
   /**
    * Creates a new action environment. The order in which the environments are combined is
    * undefined, so callers need to take care that the key set of the {@code fixedEnv} map and the
    * set of {@code inheritedEnv} elements are disjoint.
    */
-  public ActionEnvironment(Map<String, String> fixedEnv, Set<String> inheritedEnv) {
-    this.fixedEnv = ImmutableMap.copyOf(fixedEnv);
-    this.inheritedEnv = ImmutableSet.copyOf(inheritedEnv);
+  public static ActionEnvironment create(Map<String, String> fixedEnv, Set<String> inheritedEnv) {
+    if (fixedEnv.isEmpty() && inheritedEnv.isEmpty()) {
+      return EMPTY;
+    }
+    return new ActionEnvironment(fixedEnv, inheritedEnv);
   }
 
-  public ActionEnvironment(Map<String, String> fixedEnv) {
-    this(fixedEnv, ImmutableSet.of());
+  public static ActionEnvironment create(Map<String, String> fixedEnv) {
+    return new ActionEnvironment(fixedEnv, ImmutableSet.of());
   }
 
   public ImmutableMap<String, String> getFixedEnv() {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
index 0cf88d3074..8ca651b757 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
@@ -648,7 +648,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
       if (useDefaultShellEnvironment) {
         env = Preconditions.checkNotNull(configEnv);
       } else {
-        env = new ActionEnvironment(this.environment);
+        env = ActionEnvironment.create(this.environment);
       }
 
       if (disableSandboxing) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java
index 024ea9feaa..89179476ca 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java
@@ -79,6 +79,10 @@ public final class JavaCompileAction extends SpawnAction {
   static final ImmutableMap<String, String> UTF8_ENVIRONMENT =
       ImmutableMap.of("LC_CTYPE", "en_US.UTF-8");
 
+  // TODO(#3320): This is missing the configuration's action environment!
+  static final ActionEnvironment UTF8_ACTION_ENVIRONMENT =
+      ActionEnvironment.create(UTF8_ENVIRONMENT);
+
   private final CommandLine javaCompileCommandLine;
   private final CommandLine commandLine;
 
@@ -195,7 +199,7 @@ public final class JavaCompileAction extends SpawnAction {
         commandLine,
         false,
         // TODO(#3320): This is missing the configuration's action environment!
-        new ActionEnvironment(UTF8_ENVIRONMENT),
+        UTF8_ACTION_ENVIRONMENT,
         ImmutableMap.copyOf(executionInfo),
         progressMessage,
         EmptyRunfilesSupplier.INSTANCE,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java
index 93f1da06bc..0f1890111d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java
@@ -24,7 +24,6 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.Action;
-import com.google.devtools.build.lib.actions.ActionEnvironment;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionInput;
 import com.google.devtools.build.lib.actions.ActionOwner;
@@ -113,7 +112,7 @@ public class JavaHeaderCompileAction extends SpawnAction {
         transitiveCommandLine,
         false,
         // TODO(#3320): This is missing the config's action environment.
-        new ActionEnvironment(JavaCompileAction.UTF8_ENVIRONMENT),
+        JavaCompileAction.UTF8_ACTION_ENVIRONMENT,
         progressMessage,
         "Turbine");
     this.directInputs = checkNotNull(directInputs);
@@ -436,7 +435,7 @@ public class JavaHeaderCompileAction extends SpawnAction {
               transitiveCommandLine,
               false,
               // TODO(b/63280599): This is missing the config's action environment.
-              new ActionEnvironment(JavaCompileAction.UTF8_ENVIRONMENT),
+              JavaCompileAction.UTF8_ACTION_ENVIRONMENT,
               getProgressMessageWithAnnotationProcessors(),
               "JavacTurbine")
         };
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java
index 975377a96c..ad4db70673 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java
@@ -454,7 +454,7 @@ public class ObjcCompileAction extends SpawnAction {
           actualCommandLine,
           isShellCommand,
           // TODO(#3320): This is missing the inherited action env from --action_env.
-          new ActionEnvironment(env.getFixedEnv()),
+          ActionEnvironment.create(env.getFixedEnv()),
           executionInfo,
           progressMessage,
           runfilesSupplier,