aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGravatar jmmv <jmmv@google.com>2018-02-12 11:18:50 -0800
committerGravatar Copybara-Service <copybara-piper@google.com>2018-02-12 11:21:07 -0800
commitd6fe763eb55e03cd7f8917a9c49e157b52355402 (patch)
tree225af3728cf0f23cb4b043f2bd34c01f983276c0
parenta70827dc07194c26458bbb9f297e26ea99fadd16 (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.java39
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();