2 files changed, 45 insertions, 26 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index f41ff41836..6cc9d11f78 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -26,6 +26,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedMap;
+import com.google.common.collect.ImmutableSortedSet;
 import com.google.common.collect.Interner;
 import com.google.common.collect.Interners;
 import com.google.common.collect.Iterables;
@@ -46,6 +47,7 @@ import com.google.devtools.build.lib.buildeventstream.GenericBuildEvent;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
+import com.google.devtools.build.lib.concurrent.BlazeInterners;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.packages.RuleClassProvider;
@@ -109,6 +111,16 @@ import javax.annotation.Nullable;
         + "depend on it and not targets that it depends on.")
 public class BuildConfiguration implements BuildEvent {
   /**
+   * Sorts fragments by class name. This produces a stable order which, e.g., facilitates consistent
+   * output from buildMnemonic.
+   */
+  public static final Comparator<Class<? extends Fragment>> lexicalFragmentSorter =
+      Comparator.comparing(Class::getName);
+
+  private static final Interner<ImmutableSortedMap<Class<? extends Fragment>, Fragment>>
+      fragmentsInterner = BlazeInterners.newWeakInterner();
+
+  /**
    * An interface for language-specific configurations.
    *
    * <p>All implementations must be immutable and communicate this as clearly as possible (e.g.
@@ -1084,7 +1096,8 @@ public class BuildConfiguration implements BuildEvent {
 
   private final String checksum;
 
-  private final ImmutableMap<Class<? extends Fragment>, Fragment> fragments;
+  private final ImmutableSortedMap<Class<? extends Fragment>, Fragment> fragments;
+
   private final ImmutableMap<String, Class<? extends Fragment>> skylarkVisibleFragments;
   private final RepositoryName mainRepositoryName;
   private final ImmutableSet<String> reservedActionMnemonics;
@@ -1259,7 +1272,7 @@ public class BuildConfiguration implements BuildEvent {
     BuildConfiguration otherConfig = (BuildConfiguration) other;
     return actionsEnabled == otherConfig.actionsEnabled
         && fragments.values().equals(otherConfig.fragments.values())
-        && buildOptions.getOptions().equals(otherConfig.buildOptions.getOptions());
+        && buildOptions.equals(otherConfig.buildOptions);
   }
 
   private int computeHashCode() {
@@ -1347,17 +1360,10 @@ public class BuildConfiguration implements BuildEvent {
     return ActionEnvironment.split(testEnv);
   }
 
-  /**
-   * Sorts fragments by class name. This produces a stable order which, e.g., facilitates
-   * consistent output from buildMneumonic.
-   */
-  private static final Comparator lexicalFragmentSorter =
-      new Comparator<Class<? extends Fragment>>() {
-        @Override
-        public int compare(Class<? extends Fragment> o1, Class<? extends Fragment> o2) {
-          return o1.getName().compareTo(o2.getName());
-        }
-      };
+  private static ImmutableSortedMap<Class<? extends Fragment>, Fragment> makeFragmentsMap(
+      Map<Class<? extends Fragment>, Fragment> fragmentsMap) {
+    return fragmentsInterner.intern(ImmutableSortedMap.copyOf(fragmentsMap, lexicalFragmentSorter));
+  }
 
   /**
    * Constructs a new BuildConfiguration instance.
@@ -1367,7 +1373,7 @@ public class BuildConfiguration implements BuildEvent {
       BuildOptions buildOptions,
       String repositoryName) {
     this.directories = directories;
-    this.fragments = ImmutableSortedMap.copyOf(fragmentsMap, lexicalFragmentSorter);
+    this.fragments = makeFragmentsMap(fragmentsMap);
 
     this.skylarkVisibleFragments = buildIndexOfSkylarkVisibleFragments();
 
@@ -1836,10 +1842,8 @@ public class BuildConfiguration implements BuildEvent {
     return true;
   }
 
-  /**
-   * Which fragments does this configuration contain?
-   */
-  public Set<Class<? extends Fragment>> fragmentClasses() {
+  /** Which fragments does this configuration contain? */
+  public ImmutableSortedSet<Class<? extends Fragment>> fragmentClasses() {
     return fragments.keySet();
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationValue.java
index f7057d8e25..8389422fa6 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationValue.java
@@ -14,8 +14,12 @@
 package com.google.devtools.build.lib.skyframe;
 
 import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableSortedSet;
+import com.google.common.collect.Interner;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
+import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
+import com.google.devtools.build.lib.concurrent.BlazeInterners;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
 import com.google.devtools.build.skyframe.SkyFunctionName;
 import com.google.devtools.build.skyframe.SkyKey;
@@ -31,6 +35,7 @@ import java.util.Set;
 // @Immutable
 @ThreadSafe
 public class BuildConfigurationValue implements SkyValue {
+  private static final Interner<Key> keyInterner = BlazeInterners.newWeakInterner();
 
   private final BuildConfiguration configuration;
 
@@ -51,16 +56,20 @@ public class BuildConfigurationValue implements SkyValue {
   @ThreadSafe
   public static SkyKey key(Set<Class<? extends BuildConfiguration.Fragment>> fragments,
       BuildOptions buildOptions) {
-    return new Key(fragments, buildOptions);
+    return keyInterner.intern(
+        new Key(
+            ImmutableSortedSet.copyOf(BuildConfiguration.lexicalFragmentSorter, fragments),
+            buildOptions));
   }
 
   static final class Key implements SkyKey, Serializable {
-    private final Set<Class<? extends BuildConfiguration.Fragment>> fragments;
+    private final ImmutableSortedSet<Class<? extends BuildConfiguration.Fragment>> fragments;
     private final BuildOptions buildOptions;
     private final boolean enableActions;
+    // If hashCode really is -1, we'll recompute it from scratch each time. Oh well.
+    private volatile int hashCode = -1;
 
-    Key(Set<Class<? extends BuildConfiguration.Fragment>> fragments,
-        BuildOptions buildOptions) {
+    Key(ImmutableSortedSet<Class<? extends Fragment>> fragments, BuildOptions buildOptions) {
       this.fragments = fragments;
       this.buildOptions = Preconditions.checkNotNull(buildOptions);
       // Cache this value for quicker access on .equals() / .hashCode(). We don't cache it inside
@@ -69,7 +78,7 @@ public class BuildConfigurationValue implements SkyValue {
       this.enableActions = buildOptions.enableActions();
     }
 
-    Set<Class<? extends BuildConfiguration.Fragment>> getFragments() {
+    ImmutableSortedSet<Class<? extends BuildConfiguration.Fragment>> getFragments() {
       return fragments;
     }
 
@@ -84,18 +93,24 @@ public class BuildConfigurationValue implements SkyValue {
 
     @Override
     public boolean equals(Object o) {
+      if (this == o) {
+        return true;
+      }
       if (!(o instanceof Key)) {
         return false;
       }
       Key otherConfig = (Key) o;
-      return Objects.equals(fragments, otherConfig.fragments)
-          && Objects.equals(buildOptions, otherConfig.buildOptions)
+      return buildOptions.equals(otherConfig.buildOptions)
+          && Objects.equals(fragments, otherConfig.fragments)
           && enableActions == otherConfig.enableActions;
     }
 
     @Override
     public int hashCode() {
-      return Objects.hash(fragments, buildOptions, enableActions);
+      if (hashCode == -1) {
+        hashCode = Objects.hash(fragments, buildOptions, enableActions);
+      }
+      return hashCode;
     }
   }
 }