-
Notifications
You must be signed in to change notification settings - Fork 125
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use BufWriter when STDOUT is not a TTY #206
Conversation
My expectation would actually be that the standard library itself implemented this. And it seems like it's intended to, but it's a At the moment, it seems like there is no way to get a handle to a Ultimately, this means that once std fixes this behavior, this PR will be unnecessary, and performance will immediately improve. Merging this PR will also not improve performance I don't think. Providing a mechanism specifically for writing to a file would help, though I'm a little hesitant to merge a change that complicates both the internal and external API when it will ultimately become unnecessary following a change to |
It will be really nice when the standard library implements this, but I still believe this PR should help with performance. When you write more than a line to the line writer, it will write the buffer directly to the inner writer. This means if we wrap stdio in a Of course it will still call fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
let newline_idx = match memchr::memrchr(b'\n', buf) {
// If there are no new newlines (that is, if this write is less than
// one line), just do a regular buffered write (which may flush if
// we exceed the inner buffer's size)
None => {
self.flush_if_completed_line()?;
return self.buffer.write(buf);
}
// Otherwise, arrange for the lines to be written directly to the
// inner writer.
Some(newline_idx) => newline_idx + 1,
}; Once the standard library is updated to handle this, we can remove this workaround. |
Yeah, that's a good point. Happy to merge. The CI failure appears to be something broken in tarpaulin, not in our code. |
Oh, actually, before I merge, would you mind updating the changelog? |
This is an optimization to avoid line buffering when redirecting output.
Close #193