Intern trivial ActionEnvironment and EnvironmentVariables instances

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

@@ -81,10 +81,10 @@ default int size() {
  */
  static class CompoundEnvironmentVariables implements EnvironmentVariables {
 
- static EnvironmentVariables create(
- Map<String, String> fixedVars, Set<String> inheritedVars, EnvironmentVariables base) {
- if (fixedVars.isEmpty() && inheritedVars.isEmpty() && base.isEmpty()) {
- return EMPTY_ENVIRONMENT_VARIABLES;
+ static EnvironmentVariables create(Map<String, String> fixedVars, Set<String> inheritedVars,
+ EnvironmentVariables base) {
+ if (fixedVars.isEmpty() && inheritedVars.isEmpty()) {
+ return base;
  }
  return new CompoundEnvironmentVariables(fixedVars, inheritedVars, base);
  }
@@ -219,18 +219,22 @@ public static ActionEnvironment create(Map<String, String> fixedEnv) {
  * Returns a copy of the environment with the given fixed variables added to it, <em>overwriting
  * any existing occurrences of those variables</em>.
  */
- public ActionEnvironment addFixedVariables(Map<String, String> fixedVars) {
- return ActionEnvironment.create(
- CompoundEnvironmentVariables.create(fixedVars, ImmutableSet.of(), vars));
+ public ActionEnvironment withAdditionalFixedVariables(Map<String, String> fixedVars) {
+ return withAdditionalVariables(fixedVars, ImmutableSet.of());
  }
 
  /**
  * Returns a copy of the environment with the given fixed and inherited variables added to it,
  * <em>overwriting any existing occurrences of those variables</em>.
  */
- public ActionEnvironment addVariables(Map<String, String> fixedVars, Set<String> inheritedVars) {
- return ActionEnvironment.create(
- CompoundEnvironmentVariables.create(fixedVars, inheritedVars, vars));
+ public ActionEnvironment withAdditionalVariables(Map<String, String> fixedVars,
+ Set<String> inheritedVars) {
+ EnvironmentVariables newVars = CompoundEnvironmentVariables.create(fixedVars, inheritedVars,
+ vars);
+ if (newVars == vars) {
+ return this;
+ }
+ return ActionEnvironment.create(newVars);
  }
 
  /**

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

@@ -413,7 +413,7 @@ private TestParams createTestAction(int shards)
  testProperties,
  runfilesSupport
  .getActionEnvironment()
- .addVariables(extraTestEnv, extraInheritedEnv),
+ .withAdditionalVariables(extraTestEnv, extraInheritedEnv),
  executionSettings,
  shard,
  run,

src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileActionBuilder.java

@@ -205,7 +205,8 @@ public JavaCompileAction build() {
  toolsBuilder.addTransitive(toolsJars);
 
  ActionEnvironment actionEnvironment =
- ruleContext.getConfiguration().getActionEnvironment().addFixedVariables(UTF8_ENVIRONMENT);
+ ruleContext.getConfiguration().getActionEnvironment()
+ .withAdditionalFixedVariables(UTF8_ENVIRONMENT);
 
  NestedSetBuilder<Artifact> mandatoryInputsBuilder = NestedSetBuilder.stableOrder();
  mandatoryInputsBuilder

src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileActionBuilder.java

@@ -296,7 +296,8 @@ public void build(JavaToolchainProvider javaToolchain) throws InterruptedExcepti
  }
 
  ActionEnvironment actionEnvironment =
- ruleContext.getConfiguration().getActionEnvironment().addFixedVariables(UTF8_ENVIRONMENT);
+ ruleContext.getConfiguration().getActionEnvironment()
+ .withAdditionalFixedVariables(UTF8_ENVIRONMENT);
 
  OnDemandString progressMessage =
  new ProgressMessage(

src/test/java/com/google/devtools/build/lib/actions/ActionEnvironmentTest.java

@@ -34,7 +34,7 @@ public void compoundEnvOrdering() {
  ActionEnvironment.create(
  ImmutableMap.of("FOO", "foo1", "BAR", "bar"), ImmutableSet.of("baz"));
  // entries added by env2 override the existing entries
- ActionEnvironment env2 = env1.addFixedVariables(ImmutableMap.of("FOO", "foo2"));
+ ActionEnvironment env2 = env1.withAdditionalFixedVariables(ImmutableMap.of("FOO", "foo2"));
 
  assertThat(env1.getFixedEnv()).containsExactly("FOO", "foo1", "BAR", "bar");
  assertThat(env1.getInheritedEnv()).containsExactly("baz");
@@ -45,12 +45,11 @@ public void compoundEnvOrdering() {
 
  @Test
  public void fixedInheritedInteraction() {
- ActionEnvironment env =
- ActionEnvironment.create(
- ImmutableMap.of("FIXED_ONLY", "fixed"), ImmutableSet.of("INHERITED_ONLY"))
- .addVariables(
- ImmutableMap.of("FIXED_AND_INHERITED", "fixed"),
- ImmutableSet.of("FIXED_AND_INHERITED"));
+ ActionEnvironment env = ActionEnvironment.create(
+ ImmutableMap.of("FIXED_ONLY", "fixed"),
+ ImmutableSet.of("INHERITED_ONLY"))
+ .withAdditionalVariables(ImmutableMap.of("FIXED_AND_INHERITED", "fixed"),
+ ImmutableSet.of("FIXED_AND_INHERITED"));
  Map<String, String> clientEnv =
  ImmutableMap.of("INHERITED_ONLY", "inherited", "FIXED_AND_INHERITED", "inherited");
  Map<String, String> result = new HashMap<>();
@@ -65,4 +64,17 @@ public void fixedInheritedInteraction() {
  "INHERITED_ONLY",
  "inherited");
  }
+
+ @Test
+ public void emptyEnvironmentInterning() {
+ ActionEnvironment emptyEnvironment = ActionEnvironment.create(ImmutableMap.of(),
+ ImmutableSet.of());
+ assertThat(emptyEnvironment).isSameInstanceAs(ActionEnvironment.EMPTY);
+
+ ActionEnvironment base = ActionEnvironment.create(ImmutableMap.of("FOO", "foo1"),
+ ImmutableSet.of("baz"));
+ assertThat(base.withAdditionalFixedVariables(ImmutableMap.of())).isSameInstanceAs(base);
+ assertThat(base.withAdditionalVariables(ImmutableMap.of(), ImmutableSet.of())).isSameInstanceAs(
+ base);
+ }
 }