commit e5759a7c24b855fdb95082dffab27a6e246913de
author Googler <noreply@google.com> Mon Jun 08 02:09:40 2020 -0700
committer Copybara-Service <copybara-worker@google.com> Mon Jun 08 02:11:08 2020 -0700
tree 8f02e4061da783659d23634b63c5b65a63264618
parent c67e76b87bc018f79e1fb2fe46100e1927284667

Simplify CriticalPathComponent

This introduces `SpawnMetrics#isEmpty` and a placeholder `SpawnMetrics` in
`CriticalPathComponent` to simplify its code.

RELNOTES: None.
PiperOrigin-RevId: 315234616

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

diff --git a/src/main/java/com/google/devtools/build/lib/actions/SpawnMetrics.java b/src/main/java/com/google/devtools/build/lib/actions/SpawnMetrics.java
index f6cf871..d7836bf 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/SpawnMetrics.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/SpawnMetrics.java
@@ -89,6 +89,11 @@
     return execKind;
   }
 
+  /** Returns true if {@link #totalTime()} is zero. */
+  public boolean isEmpty() {
+    return totalTime.isZero();
+  }
+
   /**
    * Total (measured locally) wall time spent running a spawn. This should be at least as large as
    * all the other times summed together.

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

diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CriticalPathComponent.java b/src/main/java/com/google/devtools/build/lib/runtime/CriticalPathComponent.java
index b5f599c..6f45d55 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/CriticalPathComponent.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/CriticalPathComponent.java
@@ -54,6 +54,10 @@
     return (startNanos) -> nowInMillis - TimeUnit.NANOSECONDS.toMillis((nowInNanos - startNanos));
   }
 
+  /** Empty metrics used to simplify handling of {@link #phaseMaxMetrics}. */
+  private static final SpawnMetrics EMPTY_PLACEHOLDER_METRICS =
+      SpawnMetrics.Builder.forOtherExec().build();
+
   // These two fields are values of BlazeClock.nanoTime() at the relevant points in time.
   private long startNanos;
   private long finishNanos = 0;
@@ -66,7 +70,7 @@
   private final Artifact primaryOutput;
 
   /** Spawn metrics for this action. */
-  @Nullable private SpawnMetrics phaseMaxMetrics = null;
+  private SpawnMetrics phaseMaxMetrics = EMPTY_PLACEHOLDER_METRICS;
 
   private AggregatedSpawnMetrics totalSpawnMetrics = AggregatedSpawnMetrics.EMPTY;
   private Duration longestRunningTotalDuration = Duration.ZERO;
@@ -124,9 +128,9 @@
     }
 
     // If the phaseMaxMetrics has Duration, then we want to aggregate it to the total.
-    if (this.phaseMaxMetrics != null && !this.phaseMaxMetrics.totalTime().isZero()) {
+    if (!this.phaseMaxMetrics.isEmpty()) {
       this.totalSpawnMetrics = this.totalSpawnMetrics.sumDurationsMaxOther(phaseMaxMetrics);
-      this.phaseMaxMetrics = null;
+      this.phaseMaxMetrics = EMPTY_PLACEHOLDER_METRICS;
     }
   }
 
@@ -197,13 +201,12 @@
       this.remote = true;
     }
     if (this.phaseChange) {
-      if (this.phaseMaxMetrics != null) {
+      if (!this.phaseMaxMetrics.isEmpty()) {
         this.totalSpawnMetrics = this.totalSpawnMetrics.sumDurationsMaxOther(phaseMaxMetrics);
       }
       this.phaseMaxMetrics = metrics;
       this.phaseChange = false;
-    } else if (this.phaseMaxMetrics == null
-        || metrics.totalTime().compareTo(this.phaseMaxMetrics.totalTime()) > 0) {
+    } else if (metrics.totalTime().compareTo(this.phaseMaxMetrics.totalTime()) > 0) {
       this.phaseMaxMetrics = metrics;
     }