From 720d66f1b6de839d64d049e365da04fb4f98fb04 Mon Sep 17 00:00:00 2001 From: ruperts Date: Wed, 29 Nov 2017 04:05:57 -0800 Subject: Add user and system time to CommandResults, and plumb them into SpawnResults. RELNOTES: None PiperOrigin-RevId: 177290508 --- .../devtools/build/lib/actions/SpawnResult.java | 15 +++ .../build/lib/exec/local/LocalSpawnRunner.java | 13 ++- .../lib/sandbox/AbstractSandboxSpawnRunner.java | 11 +- .../java/com/google/devtools/build/lib/shell/BUILD | 6 +- .../devtools/build/lib/shell/CommandResult.java | 130 +++++++++++++++------ .../build/lib/shell/FutureCommandResultImpl.java | 24 ++-- 6 files changed, 147 insertions(+), 52 deletions(-) (limited to 'src/main/java/com') 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 wallTime) { + this.wallTime = wallTime; + return this; + } + + public Builder setUserTime(Optional userTime) { + this.userTime = userTime; + return this; + } + + public Builder setSystemTime(Optional 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 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 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 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 wallExecutionTime); + + /** Sets or clears the user execution time. */ + public abstract Builder setUserExecutionTime(Optional userExecutionTime); + + /** Sets or clears the system execution time. */ + public abstract Builder setSystemExecutionTime(Optional 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()); } } -- cgit v1.2.3