Extract part of EnvironmentGroup that's necessary for constraint calculation, so we don't have to have the whole package.

Memory increase should be minimal because there aren't that many environment groups, but it's further minimized by breaking an inner class UnpackagedEnvironmentGroup out of EnvironmentGroup. Previously, each EnvironmentGroup cost 40 bytes (24 bytes for first three fields, 8 for next two, 8 for last field because of alignment). Each UnpackagedEnvironmentGroup costs 32 bytes (4 fields), while the EnvironmentGroup now costs 24 bytes. So a loss of 16 bytes per EnvironmentGroup: shouldn't be noticeable.

PiperOrigin-RevId: 185837140

1 files changed, 31 insertions, 56 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/packages/EnvironmentGroup.java b/src/main/java/com/google/devtools/build/lib/packages/EnvironmentGroup.java
index 0536dc3a68..a0e8b23ece 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/EnvironmentGroup.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/EnvironmentGroup.java
@@ -17,7 +17,6 @@ package com.google.devtools.build.lib.packages;
 import com.google.common.base.Predicate;
 import com.google.common.base.Verify;
 import com.google.common.collect.HashMultimap;
-import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Multimap;
 import com.google.common.collect.Sets;
@@ -28,10 +27,8 @@ import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.Location;
-
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -67,18 +64,9 @@ import java.util.Set;
  */
 @Immutable
 public class EnvironmentGroup implements Target {
-  private final Label label;
+  private final EnvironmentLabels environmentLabels;
   private final Location location;
   private final Package containingPackage;
-  private final Set<Label> environments;
-  private final Set<Label> defaults;
-
-  /**
-   * Maps a member environment to the set of environments that directly fulfill it. Note that
-   * we can't populate this map until all Target instances for member environments have been
-   * initialized, which may occur after group instantiation (this makes the class mutable).
-   */
-  private final Map<Label, NestedSet<Label>> fulfillersMap = new HashMap<>();
 
   /**
    * Predicate that matches labels from a different package than the initialized package.
@@ -106,13 +94,19 @@ public class EnvironmentGroup implements Target {
    * @param defaults the environments a rule implicitly supports unless otherwise specified
    * @param location location in the BUILD file of this group
    */
-  EnvironmentGroup(Label label, Package pkg, final List<Label> environments, List<Label> defaults,
+  EnvironmentGroup(
+      Label label,
+      Package pkg,
+      final List<Label> environments,
+      List<Label> defaults,
       Location location) {
-    this.label = label;
+    this.environmentLabels = new EnvironmentLabels(label, environments, defaults);
     this.location = location;
     this.containingPackage = pkg;
-    this.environments = ImmutableSet.copyOf(environments);
-    this.defaults = ImmutableSet.copyOf(defaults);
+  }
+
+  public EnvironmentLabels getEnvironmentLabels() {
+    return environmentLabels;
   }
 
   /**
@@ -130,13 +124,16 @@ public class EnvironmentGroup implements Target {
 
     // All environments should belong to the same package as this group.
     for (Label environment :
-        Iterables.filter(environments, new DifferentPackage(containingPackage))) {
-      events.add(Event.error(location,
-          environment + " is not in the same package as group " + label));
+        Iterables.filter(environmentLabels.environments, new DifferentPackage(containingPackage))) {
+      events.add(
+          Event.error(
+              location,
+              environment + " is not in the same package as group " + environmentLabels.label));
     }
 
     // The defaults must be a subset of the member environments.
-    for (Label unknownDefault : Sets.difference(defaults, environments)) {
+    for (Label unknownDefault :
+        Sets.difference(environmentLabels.defaults, environmentLabels.environments)) {
       events.add(Event.error(location, "default " + unknownDefault + " is not a "
           + "declared environment for group " + getLabel()));
     }
@@ -145,9 +142,9 @@ public class EnvironmentGroup implements Target {
   }
 
   /**
-   * Checks that the group's declared environments are legitimate same-package environment
-   * rules and prepares the "fulfills" relationships between these environments to support
-   * {@link #getFulfillers}.
+   * Checks that the group's declared environments are legitimate same-package environment rules and
+   * prepares the "fulfills" relationships between these environments to support {@link
+   * EnvironmentLabels#getFulfillers}.
    *
    * @param pkgTargets mapping from label name to target instance for this group's package
    * @return a list of validation errors that occurred
@@ -157,7 +154,7 @@ public class EnvironmentGroup implements Target {
     // Maps an environment to the environments that directly fulfill it.
     Multimap<Label, Label> directFulfillers = HashMultimap.create();
 
-    for (Label envName : environments) {
+    for (Label envName : environmentLabels.environments) {
       Target env = pkgTargets.get(envName.getName());
       if (isValidEnvironment(env, envName, "", events)) {
         AttributeMap attr = NonconfigurableAttributeMapper.of((Rule) env);
@@ -174,9 +171,9 @@ public class EnvironmentGroup implements Target {
     // transitively fulfill each other. We could alternatively compute this on-demand, but since
     // we don't expect these chains to be very large we opt toward computing them once at package
     // load time.
-    Verify.verify(fulfillersMap.isEmpty());
-    for (Label envName : environments) {
-      setTransitiveFulfillers(envName, directFulfillers, fulfillersMap);
+    Verify.verify(environmentLabels.fulfillersMap.isEmpty());
+    for (Label envName : environmentLabels.environments) {
+      setTransitiveFulfillers(envName, directFulfillers, environmentLabels.fulfillersMap);
     }
 
     return events;
@@ -216,7 +213,7 @@ public class EnvironmentGroup implements Target {
     } else if (!env.getTargetKind().equals("environment rule")) {
       events.add(Event.error(location, prefix + env.getLabel() + " is not a valid environment"));
       return false;
-    } else if (!environments.contains(env.getLabel())) {
+    } else if (!environmentLabels.environments.contains(env.getLabel())) {
       events.add(Event.error(location, prefix + env.getLabel() + " is not a member of this group"));
       return false;
     }
@@ -227,7 +224,7 @@ public class EnvironmentGroup implements Target {
    * Returns the environments that belong to this group.
    */
   public Set<Label> getEnvironments() {
-    return environments;
+    return environmentLabels.environments;
   }
 
   /**
@@ -235,39 +232,17 @@ public class EnvironmentGroup implements Target {
    * environments in this group.
    */
   public Set<Label> getDefaults() {
-    return defaults;
-  }
-
-  /**
-   * Determines whether or not an environment is a default. Returns false if the environment
-   * doesn't belong to this group.
-   */
-  public boolean isDefault(Label environment) {
-    return defaults.contains(environment);
-  }
-
-  /**
-   * Returns the set of environments that transitively fulfill the specified environment.
-   * The environment must be a valid member of this group.
-   *
-   * <p>>For example, if the input is <code>":foo"</code> and <code>":bar"</code> fulfills
-   * <code>":foo"</code> and <code>":baz"</code> fulfills <code>":bar"</code>, this returns
-   * <code>[":foo", ":bar", ":baz"]</code>.
-   *
-   * <p>If no environments fulfill the input, returns an empty set.
-   */
-  public Iterable<Label> getFulfillers(Label environment) {
-    return Verify.verifyNotNull(fulfillersMap.get(environment));
+    return environmentLabels.defaults;
   }
 
   @Override
   public Label getLabel() {
-    return label;
+    return environmentLabels.label;
   }
 
   @Override
   public String getName() {
-    return label.getName();
+    return environmentLabels.label.getName();
   }
 
   @Override
@@ -297,7 +272,7 @@ public class EnvironmentGroup implements Target {
 
   @Override
   public String toString() {
-   return targetKind() + " " + getLabel();
+    return targetKind() + " " + getLabel();
   }
 
   @Override