Skip to content

Commit 7a0cde9

Browse files
committed
Auto merge of rust-lang#134620 - ChrisDenton:line-writer, r=tgross35
Avoid short writes in LineWriter If the bytes written to `LineWriter` contains at least one new line but doesn't end in a new line (e.g. `"abc\ndef"`) then we: - write up to the last new line direct to the underlying `Writer`. - copy as many of the remaining bytes as will fit into our internal buffer. That last step is inefficient if the remaining bytes are larger than our buffer. It will needlessly split the bytes in two, requiring at least two writes to the underlying `Writer` (one to flush the buffer, one more to write the rest). This PR skips the extra buffering if the remaining bytes are larger than the buffer.
2 parents aea4e43 + fdb43ef commit 7a0cde9

File tree

2 files changed

+21
-6
lines changed

2 files changed

+21
-6
lines changed

Diff for: library/std/src/io/buffered/linewritershim.rs

+8-1
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,14 @@ impl<'a, W: ?Sized + Write> Write for LineWriterShim<'a, W> {
119119
// the buffer?
120120
// - If not, scan for the last newline that *does* fit in the buffer
121121
let tail = if flushed >= newline_idx {
122-
&buf[flushed..]
122+
let tail = &buf[flushed..];
123+
// Avoid unnecessary short writes by not splitting the remaining
124+
// bytes if they're larger than the buffer.
125+
// They can be written in full by the next call to write.
126+
if tail.len() >= self.buffer.capacity() {
127+
return Ok(flushed);
128+
}
129+
tail
123130
} else if newline_idx - flushed <= self.buffer.capacity() {
124131
&buf[flushed..newline_idx]
125132
} else {

Diff for: library/std/src/io/buffered/tests.rs

+13-5
Original file line numberDiff line numberDiff line change
@@ -847,22 +847,30 @@ fn long_line_flushed() {
847847
}
848848

849849
/// Test that, given a very long partial line *after* successfully
850-
/// flushing a complete line, the very long partial line is buffered
851-
/// unconditionally, and no additional writes take place. This assures
850+
/// flushing a complete line, no additional writes take place. This assures
852851
/// the property that `write` should make at-most-one attempt to write
853852
/// new data.
854853
#[test]
855854
fn line_long_tail_not_flushed() {
856855
let writer = ProgrammableSink::default();
857856
let mut writer = LineWriter::with_capacity(5, writer);
858857

859-
// Assert that Line 1\n is flushed, and 01234 is buffered
860-
assert_eq!(writer.write(b"Line 1\n0123456789").unwrap(), 12);
858+
// Assert that Line 1\n is flushed and the long tail isn't.
859+
let bytes = b"Line 1\n0123456789";
860+
writer.write(bytes).unwrap();
861861
assert_eq!(&writer.get_ref().buffer, b"Line 1\n");
862+
}
863+
864+
// Test that appending to a full buffer emits a single write, flushing the buffer.
865+
#[test]
866+
fn line_full_buffer_flushed() {
867+
let writer = ProgrammableSink::default();
868+
let mut writer = LineWriter::with_capacity(5, writer);
869+
assert_eq!(writer.write(b"01234").unwrap(), 5);
862870

863871
// Because the buffer is full, this subsequent write will flush it
864872
assert_eq!(writer.write(b"5").unwrap(), 1);
865-
assert_eq!(&writer.get_ref().buffer, b"Line 1\n01234");
873+
assert_eq!(&writer.get_ref().buffer, b"01234");
866874
}
867875

868876
/// Test that, if an attempt to pre-flush buffered data returns Ok(0),

0 commit comments

Comments
 (0)