Use the execution time instead of the worker time for the wall time

SpawnResult.getWallTime() is documented as follows: Returns the wall time taken by the {@link Spawn}'s execution. With the math performed as is, we also end up including time spent downloading inputs and uploading outputs. I don't think that this is in the spirit of this method. There are also getUserTime() and getSystemTime(), meaning this was likely modeled after the time(1) utility. Cc @tjgq, as this logic was added in 93029d8. Closes #16487. Change-Id: Id8683e2109ce2803877eea535da56d13ae77afc7 PiperOrigin-RevId: 481609769
bazelbuild · Oct 17, 2022 · a624e21 · a624e21
1 parent 2876569
commit a624e21
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 6 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
@@ -129,8 +129,8 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context)
                   /*cacheHit=*/ true,
                   result.cacheName(),
                   inMemoryOutput,
-                  result.getExecutionMetadata().getWorkerStartTimestamp(),
-                  result.getExecutionMetadata().getWorkerCompletedTimestamp(),
+                  result.getExecutionMetadata().getExecutionStartTimestamp(),
+                  result.getExecutionMetadata().getExecutionCompletedTimestamp(),
                   spawnMetrics.build(),
                   spawn.getMnemonic());
           return SpawnCache.success(spawnResult);

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
@@ -424,8 +424,8 @@ private SpawnResult downloadAndFinalizeSpawnResult(
         cacheHit,
         cacheName,
         inMemoryOutput,
-        result.getExecutionMetadata().getWorkerStartTimestamp(),
-        result.getExecutionMetadata().getWorkerCompletedTimestamp(),
+        result.getExecutionMetadata().getExecutionStartTimestamp(),
+        result.getExecutionMetadata().getExecutionCompletedTimestamp(),
         spawnMetrics
             .setFetchTime(fetchTime.elapsed().minus(networkTimeEnd.minus(networkTimeStart)))
             .setTotalTime(totalTime.elapsed())

diff --git a/src/main/java/com/google/devtools/build/lib/remote/UploadManifest.java b/src/main/java/com/google/devtools/build/lib/remote/UploadManifest.java
@@ -124,10 +124,14 @@ public static UploadManifest create(
     }
 
     if (startTime.isPresent() && wallTime.isPresent()) {
+      Timestamp startTimestamp = instantToTimestamp(startTime.get());
+      Timestamp completedTimestamp = instantToTimestamp(startTime.get().plus(wallTime.get()));
       result
           .getExecutionMetadataBuilder()
-          .setWorkerStartTimestamp(instantToTimestamp(startTime.get()))
-          .setWorkerCompletedTimestamp(instantToTimestamp(startTime.get().plus(wallTime.get())));
+          .setWorkerStartTimestamp(startTimestamp)
+          .setExecutionStartTimestamp(startTimestamp)
+          .setExecutionCompletedTimestamp(completedTimestamp)
+          .setWorkerCompletedTimestamp(completedTimestamp);
     }
 
     return manifest;