diff options
author | jmmv <jmmv@google.com> | 2018-02-12 11:18:50 -0800 |
---|---|---|
committer | Copybara-Service <copybara-piper@google.com> | 2018-02-12 11:21:07 -0800 |
commit | d6fe763eb55e03cd7f8917a9c49e157b52355402 (patch) | |
tree | 225af3728cf0f23cb4b043f2bd34c01f983276c0 | |
parent | a70827dc07194c26458bbb9f297e26ea99fadd16 (diff) |
Fix screen flicker caused by small writes.
ExperimentalEventHandler is in charge of composing the pretty progress bar
that Bazel displays. This progress bar is a multi-line message with control
characters printed on the terminal.
The progress bar was composed by issuing many individual writes to an
AnsiTerminal. Because the AnsiTerminal in this case was backed by an error
stream (which are unbuffered), each of these writes resulted in a gRPC to
the Bazel client to write the message to the console. gRPC calls are much
more expensive than calls to a file descriptor, and, in general, even small
writes to a file descriptor should be avoided when preparing long messages.
To fix this, fully buffer the output messages sent to the AnsiTerminal until
explicitly flushed. ExperimentalEventHandler was already doing the right
thing regarding flushes but did not account for the fact that each write
would be (unintentionally) sent directly to the terminal.
The flicker was significant: on a pathological case (building sandboxfs with
Bazel on my MacBook Pro 13" on macOS), this change shaves about 5 seconds of
build time on the previous 45 second-long build. I think this only happened
with "bazel run" and "bazel test" invocations and not "bazel build", but
I haven't really confirmed this.
RELNOTES: None.
PiperOrigin-RevId: 185405892
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/runtime/ExperimentalEventHandler.java | 39 |
1 files changed, 38 insertions, 1 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/ExperimentalEventHandler.java b/src/main/java/com/google/devtools/build/lib/runtime/ExperimentalEventHandler.java index 1e32df901e..863cf0fea6 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/ExperimentalEventHandler.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/ExperimentalEventHandler.java @@ -46,6 +46,7 @@ import com.google.devtools.build.lib.util.io.LoggingTerminalWriter; import com.google.devtools.build.lib.util.io.OutErr; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.view.test.TestStatus.BlazeTestStatus; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.OutputStream; import java.nio.charset.StandardCharsets; @@ -167,6 +168,38 @@ public class ExperimentalEventHandler implements EventHandler { } } + /** + * An output stream that wraps another output stream and that fully buffers writes until flushed. + */ + private static class FullyBufferedOutputStream extends ByteArrayOutputStream { + /** The (possibly unbuffered) stream wrapped by this one. */ + private final OutputStream wrapped; + + /** + * Constructs a new fully-buffered output stream that wraps an unbuffered one. + * + * @param wrapped the (possibly unbuffered) stream wrapped by this one + */ + FullyBufferedOutputStream(OutputStream wrapped) { + this.wrapped = wrapped; + } + + @Override + public void flush() throws IOException { + super.flush(); + try { + writeTo(wrapped); + wrapped.flush(); + } finally { + // If we failed to write our current buffered contents to the output, there is not much + // we can do because reporting an error would require another write, and that write would + // probably fail. So, instead, we silently discard whatever was previously buffered in the + // hopes that the data itself was what caused the problem. + reset(); + } + } + } + public ExperimentalEventHandler( OutErr outErr, BlazeCommandEventHandler.Options options, Clock clock) { this.outputLimit = options.experimentalUiLimitConsoleOutput; @@ -181,7 +214,11 @@ public class ExperimentalEventHandler implements EventHandler { this.outErr = outErr; } this.cursorControl = options.useCursorControl(); - this.terminal = new AnsiTerminal(this.outErr.getErrorStream()); + // This class constructs complex, multi-line ANSI terminal sequences. Sending each individual + // write to the console directly causes flicker, so we use a fully-buffered stream to ensure + // each update is sent with just one write. (This means that the implementation in this file + // must be explicit about calling terminal.flush() every time an update has to be presented.) + this.terminal = new AnsiTerminal(new FullyBufferedOutputStream(this.outErr.getErrorStream())); this.terminalWidth = (options.terminalColumns > 0 ? options.terminalColumns : 80); this.showProgress = options.showProgress; this.progressInTermTitle = options.progressInTermTitle && options.useCursorControl(); |