Skip to content

Commit

Permalink
Auto merge of #134620 - ChrisDenton:line-writer, r=tgross35
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bors committed Dec 31, 2024
2 parents aea4e43 + fdb43ef commit 7a0cde9
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 6 deletions.
9 changes: 8 additions & 1 deletion library/std/src/io/buffered/linewritershim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,14 @@ impl<'a, W: ?Sized + Write> Write for LineWriterShim<'a, W> {
// the buffer?
// - If not, scan for the last newline that *does* fit in the buffer
let tail = if flushed >= newline_idx {
&buf[flushed..]
let tail = &buf[flushed..];
// Avoid unnecessary short writes by not splitting the remaining
// bytes if they're larger than the buffer.
// They can be written in full by the next call to write.
if tail.len() >= self.buffer.capacity() {
return Ok(flushed);
}
tail
} else if newline_idx - flushed <= self.buffer.capacity() {
&buf[flushed..newline_idx]
} else {
Expand Down
18 changes: 13 additions & 5 deletions library/std/src/io/buffered/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -847,22 +847,30 @@ fn long_line_flushed() {
}

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

// Assert that Line 1\n is flushed, and 01234 is buffered
assert_eq!(writer.write(b"Line 1\n0123456789").unwrap(), 12);
// Assert that Line 1\n is flushed and the long tail isn't.
let bytes = b"Line 1\n0123456789";
writer.write(bytes).unwrap();
assert_eq!(&writer.get_ref().buffer, b"Line 1\n");
}

// Test that appending to a full buffer emits a single write, flushing the buffer.
#[test]
fn line_full_buffer_flushed() {
let writer = ProgrammableSink::default();
let mut writer = LineWriter::with_capacity(5, writer);
assert_eq!(writer.write(b"01234").unwrap(), 5);

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

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

0 comments on commit 7a0cde9

Please sign in to comment.