Use SkyKeyInterner for ArtifactNestedSetKey and intern all instances.

This was likely missed during the `SkyKeyInterner` rollout since it uses `MapMaker` instead of an `Interner`. The original intention for that was to avoid the garbage of creating a key when one is already present. The additional garbage is more than offset by the memory savings in this change.

PiperOrigin-RevId: 531246598
Change-Id: I2c1710c6931f6e59da02dafd2bc1698eb8472675

Author: justinhorvitz

src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetFunction.java

@@ -16,12 +16,11 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 
 import com.google.common.base.Suppliers;
-import com.google.common.collect.MapMaker;
+import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.actions.ActionExecutionException;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
-import com.google.devtools.build.lib.concurrent.BlazeInterners;
 import com.google.devtools.build.lib.skyframe.ArtifactFunction.SourceArtifactException;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.skyframe.SkyFunction;
@@ -82,17 +81,6 @@ final class ArtifactNestedSetFunction implements SkyFunction {
    */
   private ConcurrentMap<SkyKey, SkyValue> artifactSkyKeyToSkyValue = new ConcurrentHashMap<>();
 
-  /**
-   * Maps the NestedSets' underlying objects to the corresponding SkyKey. This is to avoid
-   * re-creating SkyKey for the same nested set upon reevaluation because of e.g. a missing value.
-   *
-   * <p>The map weakly references its values: when the ArtifactNestedSetKey becomes otherwise
-   * unreachable, the entry is collected.
-   */
-  // Note: Not using a caffeine cache here because it used more memory (b/193294367).
-  private final ConcurrentMap<NestedSet.Node, ArtifactNestedSetKey> nestedSetToSkyKey =
-      new MapMaker().concurrencyLevel(BlazeInterners.concurrencyLevel()).weakValues().makeMap();
-
   private final Supplier<ArtifactNestedSetValue> valueSupplier;
 
   private static ArtifactNestedSetFunction singleton = null;
@@ -162,17 +150,15 @@ public SkyValue compute(SkyKey skyKey, Environment env)
   }
 
   private List<SkyKey> getDepSkyKeys(ArtifactNestedSetKey skyKey) {
-    List<Artifact> leaves = skyKey.getSet().getLeaves();
-    List<NestedSet<Artifact>> nonLeaves = skyKey.getSet().getNonLeaves();
+    ImmutableList<Artifact> leaves = skyKey.getSet().getLeaves();
+    ImmutableList<NestedSet<Artifact>> nonLeaves = skyKey.getSet().getNonLeaves();
 
     List<SkyKey> keys = new ArrayList<>(leaves.size() + nonLeaves.size());
     for (Artifact file : leaves) {
       keys.add(Artifact.key(file));
     }
     for (NestedSet<Artifact> nonLeaf : nonLeaves) {
-      keys.add(
-          nestedSetToSkyKey.computeIfAbsent(
-              nonLeaf.toNode(), node -> new ArtifactNestedSetKey(nonLeaf, node)));
+      keys.add(ArtifactNestedSetKey.create(nonLeaf));
     }
     return keys;
   }

src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetKey.java

@@ -18,28 +18,31 @@
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.skyframe.ExecutionPhaseSkyKey;
 import com.google.devtools.build.skyframe.SkyFunctionName;
+import com.google.devtools.build.skyframe.SkyKey;
+import com.google.devtools.build.skyframe.SkyKey.SkyKeyInterner;
 
 /** SkyKey for {@code NestedSet<Artifact>}. */
 public final class ArtifactNestedSetKey implements ExecutionPhaseSkyKey {
 
-  // TODO(jhorvitz): Consider sharing the nestedSetToSkyKey map in ArtifactNestedSetFunction.
+  private static final SkyKeyInterner<ArtifactNestedSetKey> interner = SkyKey.newInterner();
+
   public static ArtifactNestedSetKey create(NestedSet<Artifact> set) {
-    return new ArtifactNestedSetKey(set, set.toNode());
+    return interner.intern(new ArtifactNestedSetKey(set, set.toNode()));
   }
 
   private final NestedSet<Artifact> set;
   private final NestedSet.Node node;
 
+  private ArtifactNestedSetKey(NestedSet<Artifact> set, NestedSet.Node node) {
+    this.set = set;
+    this.node = node;
+  }
+
   @Override
   public SkyFunctionName functionName() {
     return SkyFunctions.ARTIFACT_NESTED_SET;
   }
 
-  ArtifactNestedSetKey(NestedSet<Artifact> set, NestedSet.Node node) {
-    this.set = set;
-    this.node = node;
-  }
-
   /** Returns the set of artifacts that this key represents. */
   public NestedSet<Artifact> getSet() {
     return set;
@@ -52,6 +55,11 @@ public boolean valueIsShareable() {
     return false;
   }
 
+  @Override
+  public SkyKeyInterner<?> getSkyKeyInterner() {
+    return interner;
+  }
+
   @Override
   public int hashCode() {
     return node.hashCode();