Accept Durations in the CriticalPathComponent and Profiler.

RELNOTES: None.
PiperOrigin-RevId: 186658512

9 files changed, 58 insertions, 45 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/profiler/AutoProfiler.java b/src/main/java/com/google/devtools/build/lib/profiler/AutoProfiler.java
index 97e6147119..555613da83 100644
--- a/src/main/java/com/google/devtools/build/lib/profiler/AutoProfiler.java
+++ b/src/main/java/com/google/devtools/build/lib/profiler/AutoProfiler.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.profiler;
 import com.google.common.base.Preconditions;
 import com.google.devtools.build.lib.clock.BlazeClock;
 import com.google.devtools.build.lib.clock.Clock;
+import java.time.Duration;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
@@ -344,8 +345,9 @@ public class AutoProfiler implements AutoCloseable {
     @Override
     public void accept(long elapsedTimeNanos) {
       if (elapsedTimeNanos > 0) {
-        Profiler.instance().logSimpleTaskDuration(startTimeNanos, elapsedTimeNanos,
-            profilerTaskType, object);
+        Profiler.instance()
+            .logSimpleTaskDuration(
+                startTimeNanos, Duration.ofNanos(elapsedTimeNanos), profilerTaskType, object);
       }
     }
   }
diff --git a/src/main/java/com/google/devtools/build/lib/profiler/Profiler.java b/src/main/java/com/google/devtools/build/lib/profiler/Profiler.java
index f56afc8f9b..0b126555d3 100644
--- a/src/main/java/com/google/devtools/build/lib/profiler/Profiler.java
+++ b/src/main/java/com/google/devtools/build/lib/profiler/Profiler.java
@@ -30,6 +30,7 @@ import java.io.DataOutputStream;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.nio.ByteBuffer;
+import java.time.Duration;
 import java.util.ArrayList;
 import java.util.IdentityHashMap;
 import java.util.List;
@@ -779,41 +780,38 @@ public final class Profiler {
   }
 
   /**
-   * Used externally to submit simple task (one that does not have any
-   * subtasks). Depending on the minDuration attribute of the task type, task
-   * may be just aggregated into the parent task and not stored directly.
+   * Used externally to submit simple task (one that does not have any subtasks). Depending on the
+   * minDuration attribute of the task type, task may be just aggregated into the parent task and
+   * not stored directly.
    *
-   * <p>Note that start and stop time must both be acquired from the same clock
-   * instance.
+   * <p>Note that start and stop time must both be acquired from the same clock instance.
    *
-   * @param startTime task start time
-   * @param stopTime task stop time
+   * @param startTimeNanos task start time
+   * @param stopTimeNanos task stop time
    * @param type task type
-   * @param object object associated with that task. Can be String object that
-   *               describes it.
+   * @param object object associated with that task. Can be String object that describes it.
    */
-  public void logSimpleTask(long startTime, long stopTime, ProfilerTask type, Object object) {
+  public void logSimpleTask(
+      long startTimeNanos, long stopTimeNanos, ProfilerTask type, Object object) {
     if (isActive() && isProfiling(type)) {
-      logTask(startTime, stopTime - startTime, type, object);
+      logTask(startTimeNanos, stopTimeNanos - startTimeNanos, type, object);
     }
   }
 
   /**
-   * Used externally to submit simple task (one that does not have any
-   * subtasks). Depending on the minDuration attribute of the task type, task
-   * may be just aggregated into the parent task and not stored directly.
+   * Used externally to submit simple task (one that does not have any subtasks). Depending on the
+   * minDuration attribute of the task type, task may be just aggregated into the parent task and
+   * not stored directly.
    *
-   * @param startTime task start time (obtained through {@link
-   *        Profiler#nanoTimeMaybe()})
+   * @param startTimeNanos task start time (obtained through {@link Profiler#nanoTimeMaybe()})
    * @param duration the duration of the task
    * @param type task type
-   * @param object object associated with that task. Can be String object that
-   *               describes it.
+   * @param object object associated with that task. Can be String object that describes it.
    */
-  public void logSimpleTaskDuration(long startTime, long duration, ProfilerTask type,
-                                    Object object) {
+  public void logSimpleTaskDuration(
+      long startTimeNanos, Duration duration, ProfilerTask type, Object object) {
     if (isActive() && isProfiling(type)) {
-      logTask(startTime, duration, type, object);
+      logTask(startTimeNanos, duration.toNanos(), type, object);
     }
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/AbstractCriticalPathComponent.java b/src/main/java/com/google/devtools/build/lib/runtime/AbstractCriticalPathComponent.java
index e4969fd393..15fb47eca1 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/AbstractCriticalPathComponent.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/AbstractCriticalPathComponent.java
@@ -19,6 +19,7 @@ import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.clock.Clock;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
+import java.time.Duration;
 import java.util.concurrent.TimeUnit;
 import javax.annotation.Nullable;
 
@@ -126,8 +127,8 @@ public class AbstractCriticalPathComponent<C extends AbstractCriticalPathCompone
   }
 
   /** To be used only in debugging: skips state invariance checks to avoid crash-looping. */
-  protected long getElapsedTimeMillisNoCheck() {
-    return TimeUnit.NANOSECONDS.toMillis(getElapsedTimeNanosNoCheck());
+  protected Duration getElapsedTimeNoCheck() {
+    return Duration.ofNanos(getElapsedTimeNanosNoCheck());
   }
 
   private long getElapsedTimeNanosNoCheck() {
@@ -135,12 +136,12 @@ public class AbstractCriticalPathComponent<C extends AbstractCriticalPathCompone
   }
 
   /**
-   * Returns the current critical path for the action in nanoseconds.
+   * Returns the current critical path for the action.
    *
    * <p>Critical path is defined as : action_execution_time + max(child_critical_path).
    */
-  public long getAggregatedElapsedTimeMillis() {
-    return TimeUnit.NANOSECONDS.toMillis(getAggregatedElapsedTimeNanos());
+  public Duration getAggregatedElapsedTime() {
+    return Duration.ofNanos(getAggregatedElapsedTimeNanos());
   }
 
   long getAggregatedElapsedTimeNanos() {
@@ -171,7 +172,7 @@ public class AbstractCriticalPathComponent<C extends AbstractCriticalPathCompone
   public String toString() {
     String currentTime = "still running ";
     if (!isRunning) {
-      currentTime = String.format("%.2f", getElapsedTimeMillisNoCheck() / 1000.0) + "s ";
+      currentTime = String.format("%.2f", getElapsedTimeNoCheck().toMillis() / 1000.0) + "s ";
     }
     return currentTime + getActionString();
   }
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/AggregatedCriticalPath.java b/src/main/java/com/google/devtools/build/lib/runtime/AggregatedCriticalPath.java
index 551b010e43..f05ce86a18 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/AggregatedCriticalPath.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/AggregatedCriticalPath.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.runtime;
 
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
+import java.time.Duration;
 
 /**
  * Aggregates all the critical path components in one object. This allows us to easily access the
@@ -23,16 +24,16 @@ import com.google.common.collect.ImmutableList;
  */
 public class AggregatedCriticalPath<T extends AbstractCriticalPathComponent<?>> {
 
-  private final long totalTime;
+  private final Duration totalTime;
   private final ImmutableList<T> criticalPathComponents;
 
-  protected AggregatedCriticalPath(long totalTime, ImmutableList<T> criticalPathComponents) {
+  protected AggregatedCriticalPath(Duration totalTime, ImmutableList<T> criticalPathComponents) {
     this.totalTime = totalTime;
     this.criticalPathComponents = criticalPathComponents;
   }
 
-  /** Total wall time in ms spent running the critical path actions. */
-  public long totalTime() {
+  /** Total wall time spent running the critical path actions. */
+  public Duration totalTime() {
     return totalTime;
   }
 
@@ -56,8 +57,7 @@ public class AggregatedCriticalPath<T extends AbstractCriticalPathComponent<?>>
 
   private String toString(boolean summary) {
     StringBuilder sb = new StringBuilder("Critical Path: ");
-    double totalMillis = totalTime;
-    sb.append(String.format("%.2f", totalMillis / 1000.0));
+    sb.append(String.format("%.2f", totalTime.toMillis() / 1000.0));
     sb.append("s");
     if (summary || criticalPathComponents.isEmpty()) {
       return sb.toString();
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
index 1fbf9ed3a6..a27b9b6078 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
@@ -93,6 +93,7 @@ import java.io.IOException;
 import java.io.OutputStream;
 import java.lang.reflect.Type;
 import java.nio.charset.StandardCharsets;
+import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Date;
@@ -395,10 +396,13 @@ public final class BlazeRuntime {
       // Instead of logEvent() we're calling the low level function to pass the timings we took in
       // the launcher. We're setting the INIT phase marker so that it follows immediately the LAUNCH
       // phase.
-      profiler.logSimpleTaskDuration(execStartTimeNanos - startupTimeNanos, 0, ProfilerTask.PHASE,
+      profiler.logSimpleTaskDuration(
+          execStartTimeNanos - startupTimeNanos,
+          Duration.ZERO,
+          ProfilerTask.PHASE,
           ProfilePhase.LAUNCH.description);
-      profiler.logSimpleTaskDuration(execStartTimeNanos, 0, ProfilerTask.PHASE,
-          ProfilePhase.INIT.description);
+      profiler.logSimpleTaskDuration(
+          execStartTimeNanos, Duration.ZERO, ProfilerTask.PHASE, ProfilePhase.INIT.description);
     }
 
     if (options.memoryProfilePath != null) {
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BuildSummaryStatsModule.java b/src/main/java/com/google/devtools/build/lib/runtime/BuildSummaryStatsModule.java
index 2a1df16bf6..927355d9a9 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BuildSummaryStatsModule.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BuildSummaryStatsModule.java
@@ -31,6 +31,7 @@ import com.google.devtools.build.lib.profiler.Profiler;
 import com.google.devtools.build.lib.profiler.ProfilerTask;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.protobuf.ByteString;
+import java.time.Duration;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.logging.Logger;
@@ -106,7 +107,7 @@ public class BuildSummaryStatsModule extends BlazeModule {
           Profiler.instance()
               .logSimpleTaskDuration(
                   stat.getStartNanos(),
-                  stat.getElapsedTimeNanos(),
+                  Duration.ofNanos(stat.getElapsedTimeNanos()),
                   ProfilerTask.CRITICAL_PATH_COMPONENT,
                   stat.prettyPrintAction());
         }
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CriticalPathComputer.java b/src/main/java/com/google/devtools/build/lib/runtime/CriticalPathComputer.java
index 1c6ce03c88..968a4412c9 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/CriticalPathComputer.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/CriticalPathComputer.java
@@ -269,8 +269,10 @@ public abstract class CriticalPathComputer<C extends AbstractCriticalPathCompone
   private boolean isBiggestCriticalPath(C newCriticalPath) {
     synchronized (lock) {
       return maxCriticalPath == null
-          || maxCriticalPath.getAggregatedElapsedTimeMillis()
-          < newCriticalPath.getAggregatedElapsedTimeMillis();
+          || maxCriticalPath
+                  .getAggregatedElapsedTime()
+                  .compareTo(newCriticalPath.getAggregatedElapsedTime())
+              < 0;
     }
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/SimpleCriticalPathComputer.java b/src/main/java/com/google/devtools/build/lib/runtime/SimpleCriticalPathComputer.java
index 482c55f68c..ed9bf897bb 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/SimpleCriticalPathComputer.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/SimpleCriticalPathComputer.java
@@ -17,6 +17,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.actions.ActionKeyContext;
 import com.google.devtools.build.lib.clock.Clock;
+import java.time.Duration;
 
 /**
  * Computes the critical path during a build.
@@ -48,15 +49,15 @@ public class SimpleCriticalPathComputer
     ImmutableList.Builder<SimpleCriticalPathComponent> components = ImmutableList.builder();
     SimpleCriticalPathComponent maxCriticalPath = getMaxCriticalPath();
     if (maxCriticalPath == null) {
-      return new AggregatedCriticalPath<>(0, components.build());
+      return new AggregatedCriticalPath<>(Duration.ZERO, components.build());
     }
     SimpleCriticalPathComponent child = maxCriticalPath;
     while (child != null) {
       components.add(child);
       child = child.getChild();
     }
-    return new AggregatedCriticalPath<>(maxCriticalPath.getAggregatedElapsedTimeMillis(),
-        components.build());
+    return new AggregatedCriticalPath<>(
+        maxCriticalPath.getAggregatedElapsedTime(), components.build());
   }
 }
 
diff --git a/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java
index a8c9747c90..002453c5ed 100644
--- a/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java
@@ -31,6 +31,7 @@ import com.google.devtools.build.skyframe.NodeEntry.DirtyState;
 import com.google.devtools.build.skyframe.ParallelEvaluatorContext.EnqueueParentBehavior;
 import com.google.devtools.build.skyframe.QueryableGraph.Reason;
 import com.google.devtools.build.skyframe.SkyFunctionException.ReifiedSkyFunctionException;
+import java.time.Duration;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -349,7 +350,10 @@ public abstract class AbstractParallelEvaluator {
               evaluatorContext.getProgressReceiver().computed(skyKey, elapsedTimeNanos);
               Profiler.instance()
                   .logSimpleTaskDuration(
-                      startTime, elapsedTimeNanos, ProfilerTask.SKYFUNCTION, skyKey);
+                      startTime,
+                      Duration.ofNanos(elapsedTimeNanos),
+                      ProfilerTask.SKYFUNCTION,
+                      skyKey);
             }
           }
         } catch (final SkyFunctionException builderException) {