Add user and system time to CommandResults, and plumb them into SpawnResults.

RELNOTES: None
PiperOrigin-RevId: 177290508

6 files changed, 147 insertions, 52 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/SpawnResult.java b/src/main/java/com/google/devtools/build/lib/actions/SpawnResult.java
index c130de86c2..8b4581cfc3 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/SpawnResult.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/SpawnResult.java
@@ -313,6 +313,21 @@ public interface SpawnResult {
       return this;
     }
 
+    public Builder setWallTime(Optional<Duration> wallTime) {
+      this.wallTime = wallTime;
+      return this;
+    }
+
+    public Builder setUserTime(Optional<Duration> userTime) {
+      this.userTime = userTime;
+      return this;
+    }
+
+    public Builder setSystemTime(Optional<Duration> systemTime) {
+      this.systemTime = systemTime;
+      return this;
+    }
+
     public Builder setCacheHit(boolean cacheHit) {
       this.cacheHit = cacheHit;
       return this;
diff --git a/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java
index e99b78b48e..bfc6a8fcbb 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java
@@ -291,9 +291,9 @@ public final class LocalSpawnRunner implements SpawnRunner {
         }
 
         long startTime = System.currentTimeMillis();
-        CommandResult result = null;
+        CommandResult commandResult = null;
         try {
-          result = cmd.execute(stdOut, stdErr);
+          commandResult = cmd.execute(stdOut, stdErr);
           if (Thread.currentThread().isInterrupted()) {
             throw new InterruptedException();
           }
@@ -301,7 +301,7 @@ public final class LocalSpawnRunner implements SpawnRunner {
           if (Thread.currentThread().isInterrupted()) {
             throw new InterruptedException();
           }
-          result = e.getResult();
+          commandResult = e.getResult();
         } catch (CommandException e) {
           // At the time this comment was written, this must be a ExecFailedException encapsulating
           // an IOException from the underlying Subprocess.Factory.
@@ -318,14 +318,15 @@ public final class LocalSpawnRunner implements SpawnRunner {
               .build();
         }
         setState(State.SUCCESS);
+        // TODO(b/62588075): Calculate wall time inside commands instead?
         Duration wallTime = Duration.ofMillis(System.currentTimeMillis() - startTime);
         boolean wasTimeout =
-            result.getTerminationStatus().timedOut()
+            commandResult.getTerminationStatus().timedOut()
                 || (useProcessWrapper && wasTimeout(policy.getTimeout(), wallTime));
         int exitCode =
             wasTimeout
                 ? POSIX_TIMEOUT_EXIT_CODE
-                : result.getTerminationStatus().getRawExitCode();
+                : commandResult.getTerminationStatus().getRawExitCode();
         Status status =
             wasTimeout
                 ? Status.TIMEOUT
@@ -335,6 +336,8 @@ public final class LocalSpawnRunner implements SpawnRunner {
             .setExitCode(exitCode)
             .setExecutorHostname(hostName)
             .setWallTime(wallTime)
+            .setUserTime(commandResult.getUserExecutionTime())
+            .setSystemTime(commandResult.getSystemExecutionTime())
             .build();
       } finally {
         // Delete the temp directory tree, so the next action that this thread executes will get a
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java
index 5612c4423e..5875dad58d 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java
@@ -139,12 +139,12 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner {
         sandbox.getSandboxExecRoot().getPathFile());
 
     long startTime = System.currentTimeMillis();
-    CommandResult result;
+    CommandResult commandResult;
     try {
       if (!tmpDir.exists() && !tmpDir.createDirectory()) {
         throw new IOException(String.format("Could not create temp directory '%s'", tmpDir));
       }
-      result = cmd.execute(outErr.getOutputStream(), outErr.getErrorStream());
+      commandResult = cmd.execute(outErr.getOutputStream(), outErr.getErrorStream());
       if (Thread.currentThread().isInterrupted()) {
         throw new InterruptedException();
       }
@@ -152,7 +152,7 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner {
       if (Thread.currentThread().isInterrupted()) {
         throw new InterruptedException();
       }
-      result = e.getResult();
+      commandResult = e.getResult();
     } catch (CommandException e) {
       // At the time this comment was written, this must be a ExecFailedException encapsulating an
       // IOException from the underlying Subprocess.Factory.
@@ -165,12 +165,13 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner {
           .build();
     }
 
+    // TODO(b/62588075): Calculate wall time inside commands instead?
     Duration wallTime = Duration.ofMillis(System.currentTimeMillis() - startTime);
     boolean wasTimeout = wasTimeout(timeout, wallTime);
     int exitCode =
         wasTimeout
             ? POSIX_TIMEOUT_EXIT_CODE
-            : result.getTerminationStatus().getRawExitCode();
+            : commandResult.getTerminationStatus().getRawExitCode();
     Status status =
         wasTimeout
             ? Status.TIMEOUT
@@ -179,6 +180,8 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner {
         .setStatus(status)
         .setExitCode(exitCode)
         .setWallTime(wallTime)
+        .setUserTime(commandResult.getUserExecutionTime())
+        .setSystemTime(commandResult.getSystemExecutionTime())
         .build();
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/shell/BUILD b/src/main/java/com/google/devtools/build/lib/shell/BUILD
index 4c31224086..2432831365 100644
--- a/src/main/java/com/google/devtools/build/lib/shell/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/shell/BUILD
@@ -14,6 +14,7 @@ java_library(
     name = "shell",
     srcs = glob(["*.java"]),
     deps = [
+        "//third_party:auto_value",
         "//third_party:guava",
     ],
 )
@@ -25,5 +26,8 @@ load("//tools/build_rules:java_rules_skylark.bzl", "bootstrap_java_library")
 bootstrap_java_library(
     name = "shell-skylark",
     srcs = glob(["*.java"]),
-    jars = ["//third_party:bootstrap_guava_and_error_prone-jars"],
+    jars = [
+        "//third_party:auto_value-jars",
+        "//third_party:bootstrap_guava_and_error_prone-jars",
+    ],
 )
diff --git a/src/main/java/com/google/devtools/build/lib/shell/CommandResult.java b/src/main/java/com/google/devtools/build/lib/shell/CommandResult.java
index 4a6f4f7acc..8370b276fb 100644
--- a/src/main/java/com/google/devtools/build/lib/shell/CommandResult.java
+++ b/src/main/java/com/google/devtools/build/lib/shell/CommandResult.java
@@ -14,17 +14,19 @@
 
 package com.google.devtools.build.lib.shell;
 
-import static com.google.common.base.Preconditions.checkNotNull;
-
+import com.google.auto.value.AutoValue;
 import java.io.ByteArrayOutputStream;
+import java.time.Duration;
+import java.util.Optional;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
 /**
- * Encapsulates the results of a command execution, including exit status
- * and output to stdout and stderr.
+ * Encapsulates the results of a command execution, including exit status and output to stdout and
+ * stderr.
  */
-public final class CommandResult {
+@AutoValue
+public abstract class CommandResult {
 
   private static final Logger logger =
       Logger.getLogger("com.google.devtools.build.lib.shell.Command");
@@ -49,57 +51,119 @@ public final class CommandResult {
       }
   };
 
-  private final ByteArrayOutputStream stdout;
-  private final ByteArrayOutputStream stderr;
-  private final TerminationStatus terminationStatus;
-
-  CommandResult(final ByteArrayOutputStream stdout,
-                final ByteArrayOutputStream stderr,
-                final TerminationStatus terminationStatus) {
-    checkNotNull(stdout);
-    checkNotNull(stderr);
-    checkNotNull(terminationStatus);
-    this.stdout = stdout;
-    this.stderr = stderr;
-    this.terminationStatus = terminationStatus;
-  }
+  /** Returns the stdout {@link ByteArrayOutputStream}. */
+  public abstract ByteArrayOutputStream getStdoutStream();
+
+  /** Returns the stderr {@link ByteArrayOutputStream}. */
+  public abstract ByteArrayOutputStream getStderrStream();
 
   /**
-   * @return raw bytes that were written to stdout by the command, or
-   *  null if caller did chose to ignore output
+   * Returns the stdout as a byte array.
+   *
+   * @return raw bytes that were written to stdout by the command, or null if caller did chose to
+   *     ignore output
    * @throws IllegalStateException if output was not collected
    */
   public byte[] getStdout() {
-    return stdout.toByteArray();
+    return getStdoutStream().toByteArray();
   }
 
   /**
-   * @return raw bytes that were written to stderr by the command, or
-   *  null if caller did chose to ignore output
+   * Returns the stderr as a byte array.
+   *
+   * @return raw bytes that were written to stderr by the command, or null if caller did chose to
+   *     ignore output
    * @throws IllegalStateException if output was not collected
    */
   public byte[] getStderr() {
-    return stderr.toByteArray();
+    return getStderrStream().toByteArray();
   }
 
+  /** Returns the termination status of the subprocess. */
+  public abstract TerminationStatus getTerminationStatus();
+
   /**
-   * @return the termination status of the subprocess.
+   * Returns the wall execution time.
+   *
+   * @return the measurement, or empty in case of execution errors or when the measurement is not
+   *     implemented for the current platform
    */
-  public TerminationStatus getTerminationStatus() {
-    return terminationStatus;
-  }
+  public abstract Optional<Duration> getWallExecutionTime();
+
+  /**
+   * Returns the user execution time.
+   *
+   * @return the measurement, or empty in case of execution errors or when the measurement is not
+   *     implemented for the current platform
+   */
+  public abstract Optional<Duration> getUserExecutionTime();
+
+  /**
+   * Returns the system execution time.
+   *
+   * @return the measurement, or empty in case of execution errors or when the measurement is not
+   *     implemented for the current platform
+   */
+  public abstract Optional<Duration> getSystemExecutionTime();
 
   void logThis() {
     if (!logger.isLoggable(Level.FINER)) {
       return;
     }
-    logger.finer(terminationStatus.toString());
+    logger.finer(getTerminationStatus().toString());
 
-    if (stdout == NO_OUTPUT_COLLECTED) {
+    if (getStdoutStream() == NO_OUTPUT_COLLECTED) {
       return;
     }
-    logger.finer("Stdout: " + LogUtil.toTruncatedString(stdout.toByteArray()));
-    logger.finer("Stderr: " + LogUtil.toTruncatedString(stderr.toByteArray()));
+    logger.finer("Stdout: " + LogUtil.toTruncatedString(getStdout()));
+    logger.finer("Stderr: " + LogUtil.toTruncatedString(getStderr()));
+  }
+
+  /** Returns a new {@link CommandResult.Builder}. */
+  public static Builder builder() {
+    return new AutoValue_CommandResult.Builder();
   }
 
+  /** A builder for {@link CommandResult}s. */
+  @AutoValue.Builder
+  public abstract static class Builder {
+    /** Sets the stdout output for the command. */
+    public abstract Builder setStdoutStream(ByteArrayOutputStream stdout);
+
+    /** Sets the stderr output for the command. */
+    public abstract Builder setStderrStream(ByteArrayOutputStream stderr);
+
+    /** Sets the termination status for the command. */
+    public abstract Builder setTerminationStatus(TerminationStatus terminationStatus);
+
+    /** Sets the wall execution time. */
+    public Builder setWallExecutionTime(Duration wallExecutionTime) {
+      setWallExecutionTime(Optional.of(wallExecutionTime));
+      return this;
+    }
+
+    /** Sets the user execution time. */
+    public Builder setUserExecutionTime(Duration userExecutionTime) {
+      setUserExecutionTime(Optional.of(userExecutionTime));
+      return this;
+    }
+
+    /** Sets the system execution time. */
+    public Builder setSystemExecutionTime(Duration systemExecutionTime) {
+      setSystemExecutionTime(Optional.of(systemExecutionTime));
+      return this;
+    }
+
+    /** Sets or clears the wall execution time. */
+    public abstract Builder setWallExecutionTime(Optional<Duration> wallExecutionTime);
+
+    /** Sets or clears the user execution time. */
+    public abstract Builder setUserExecutionTime(Optional<Duration> userExecutionTime);
+
+    /** Sets or clears the system execution time. */
+    public abstract Builder setSystemExecutionTime(Optional<Duration> systemExecutionTime);
+
+    /** Builds a {@link CommandResult} object. */
+    public abstract CommandResult build();
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/shell/FutureCommandResultImpl.java b/src/main/java/com/google/devtools/build/lib/shell/FutureCommandResultImpl.java
index a8378a3343..e81cebc036 100644
--- a/src/main/java/com/google/devtools/build/lib/shell/FutureCommandResultImpl.java
+++ b/src/main/java/com/google/devtools/build/lib/shell/FutureCommandResultImpl.java
@@ -48,9 +48,11 @@ final class FutureCommandResultImpl implements FutureCommandResult {
       }
     } catch (IOException ioe) {
       CommandResult noOutputResult =
-        new CommandResult(CommandResult.EMPTY_OUTPUT,
-                          CommandResult.EMPTY_OUTPUT,
-                          status);
+          CommandResult.builder()
+              .setStdoutStream(CommandResult.EMPTY_OUTPUT)
+              .setStderrStream(CommandResult.EMPTY_OUTPUT)
+              .setTerminationStatus(status)
+              .build();
       if (status.success()) {
         // If command was otherwise successful, throw an exception about this
         throw new AbnormalTerminationException(command, noOutputResult, ioe);
@@ -67,15 +69,19 @@ final class FutureCommandResultImpl implements FutureCommandResult {
       process.close();
     }
 
-    CommandResult result = new CommandResult(
-        outErrConsumers.getAccumulatedOut(), outErrConsumers.getAccumulatedErr(), status);
-    result.logThis();
+    CommandResult commandResult =
+        CommandResult.builder()
+            .setStdoutStream(outErrConsumers.getAccumulatedOut())
+            .setStderrStream(outErrConsumers.getAccumulatedErr())
+            .setTerminationStatus(status)
+            .build();
+    commandResult.logThis();
     if (status.success()) {
-      return result;
+      return commandResult;
     } else if (status.exited()) {
-      throw new BadExitStatusException(command, result, status.toString());
+      throw new BadExitStatusException(command, commandResult, status.toString());
     } else {
-      throw new AbnormalTerminationException(command, result, status.toString());
+      throw new AbnormalTerminationException(command, commandResult, status.toString());
     }
   }