Dedupe actions for top-k worst action selection

There are several ways to do this. This seems like the least invasive:
instead of iterating over all actions in the map, we only iterate over
those where the map key matches the primary output of the action.

Alternatives considered:
- keeping an additional set of all actions; this would require an
  additional ConcurrentHashSet to contain a pointer to all actions
- deduping based on action identity using a temporary set; I'm worried
  about the additional copy involved, and this would have to be done
  for each of the three methods

PiperOrigin-RevId: 222597517

src/main/java/com/google/devtools/build/lib/runtime/CriticalPathComponent.java

@@ -16,6 +16,7 @@
 import com.google.common.base.Preconditions;
 import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.actions.ActionOwner;
+import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.SpawnMetrics;
 import com.google.devtools.build.lib.clock.Clock;
 import com.google.devtools.build.lib.cmdline.Label;
@@ -62,6 +63,8 @@ public static NanosToEpochConverter fromClock(Clock clock) {
   /** May be nulled out after finished running to allow the action to be GC'ed. */
   @Nullable protected Action action;
 
+  private final Artifact primaryOutput;
+
   /** Spawn metrics for this action. */
   private SpawnMetrics spawnMetrics = SpawnMetrics.EMPTY;
   /** An unique identifier of the component for one build execution */
@@ -75,7 +78,8 @@ public static NanosToEpochConverter fromClock(Clock clock) {
 
   public CriticalPathComponent(int id, Action action, long startNanos) {
     this.id = id;
-    this.action = action;
+    this.action = Preconditions.checkNotNull(action);
+    this.primaryOutput = action.getPrimaryOutput();
     this.startNanos = startNanos;
   }
 
@@ -97,6 +101,12 @@ public synchronized void finishActionExecution(long startNanos, long finishNanos
     }
   }
 
+  boolean isPrimaryOutput(Artifact possiblePrimaryOutput) {
+    // We know that the keys in the CriticalPathComputer are exactly the values returned from
+    // action.getPrimaryOutput(), so pointer equality is safe here.
+    return possiblePrimaryOutput == primaryOutput;
+  }
+
   /**
    * The action for which we are storing the stat. May be null if the action has finished running.
    */

src/main/java/com/google/devtools/build/lib/runtime/CriticalPathComputer.java

@@ -15,9 +15,9 @@
 package com.google.devtools.build.lib.runtime;
 
 import com.google.common.base.Preconditions;
+import com.google.common.collect.Comparators;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Maps;
-import com.google.common.collect.Ordering;
 import com.google.common.eventbus.AllowConcurrentEvents;
 import com.google.common.eventbus.Subscribe;
 import com.google.devtools.build.lib.actions.Action;
@@ -35,11 +35,13 @@
 import com.google.devtools.build.lib.clock.Clock;
 import java.time.Duration;
 import java.util.Comparator;
+import java.util.List;
 import java.util.Objects;
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.BinaryOperator;
+import java.util.stream.Stream;
 import javax.annotation.concurrent.ThreadSafe;
 
 /**
@@ -171,28 +173,36 @@ public void spawnExecuted(SpawnExecutedEvent event) {
   }
 
   /** Returns the list of components using the most memory. */
-  public ImmutableList<CriticalPathComponent> getLargestMemoryComponents() {
-    return ImmutableList.copyOf(
-        Ordering.from(
-                Comparator.comparingLong(
-                    (CriticalPathComponent c) -> c.getSpawnMetrics().memoryEstimate()))
-            .greatestOf(outputArtifactToComponent.values(), LARGEST_MEMORY_COMPONENTS_SIZE));
+  public List<CriticalPathComponent> getLargestMemoryComponents() {
+    return uniqueActions()
+        .collect(
+            Comparators.greatest(
+                LARGEST_MEMORY_COMPONENTS_SIZE,
+                Comparator.comparingLong((c) -> c.getSpawnMetrics().memoryEstimate())));
   }
 
   /** Returns the list of components with the largest input sizes. */
-  public ImmutableList<CriticalPathComponent> getLargestInputSizeComponents() {
-    return ImmutableList.copyOf(
-        Ordering.from(
-            Comparator.comparingLong(
-                (CriticalPathComponent c) -> c.getSpawnMetrics().inputBytes()))
-            .greatestOf(outputArtifactToComponent.values(), LARGEST_INPUT_SIZE_COMPONENTS_SIZE));
+  public List<CriticalPathComponent> getLargestInputSizeComponents() {
+    return uniqueActions()
+        .collect(
+            Comparators.greatest(
+                LARGEST_INPUT_SIZE_COMPONENTS_SIZE,
+                Comparator.comparingLong((c) -> c.getSpawnMetrics().inputBytes())));
   }
 
   /** Returns the list of slowest components. */
-  public ImmutableList<CriticalPathComponent> getSlowestComponents() {
-    return ImmutableList.copyOf(
-        Ordering.from(Comparator.comparingLong(CriticalPathComponent::getElapsedTimeNanos))
-            .greatestOf(outputArtifactToComponent.values(), SLOWEST_COMPONENTS_SIZE));
+  public List<CriticalPathComponent> getSlowestComponents() {
+    return uniqueActions()
+        .collect(
+            Comparators.greatest(
+                SLOWEST_COMPONENTS_SIZE,
+                Comparator.comparingLong(CriticalPathComponent::getElapsedTimeNanos)));
+  }
+
+  private Stream<CriticalPathComponent> uniqueActions() {
+    return outputArtifactToComponent.entrySet().stream()
+        .filter((e) -> e.getValue().isPrimaryOutput(e.getKey()))
+        .map((e) -> e.getValue());
   }
 
   /**