unexpand: reuse a smaller buf, perf 14%+

[oech3]

Closes? #10654

[codspeed-hq]

Merging this PR will improve performance by 14.46%

⚡ 2 improved benchmarks
✅ 300 untouched benchmarks
⏩ 42 skipped benchmarks¹

Performance Changes

	Mode	Benchmark	BASE	HEAD	Efficiency
⚡	Simulation	unexpand_large_file[10]	328.6 ms	287.1 ms	+14.46%
⚡	Simulation	unexpand_many_lines[100000]	156.8 ms	137 ms	+14.46%

---

_{Comparing oech3:unexpand-buf (9d3fc99) with main (d24bf80)}

Footnotes

1. 42 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[github-actions]

GNU testsuite comparison:

Skip an intermittent issue tests/cut/bounded-memory (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/date/resolution (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/pr/bounded-memory (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tty/tty-eof (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/tail/tail-n0f is now being skipped but was previously passing.
Congrats! The gnu test tests/rm/many-dir-entries-vs-OOM is now passing!

[github-actions]

GNU testsuite comparison:

Skip an intermittent issue tests/pr/bounded-memory (fails in this run but passes in the 'main' branch)
Note: The gnu test tests/tail/tail-n0f is now being skipped but was previously passing.

[oech3]

It seems we achieve 41.12% by reusing BufReader's buf. But I'll wait this since it has large diff.

[github-actions]

GNU testsuite comparison:

Note: The gnu test tests/tail/tail-n0f is now being skipped but was previously passing.

[oech3]

Similar change for expand dropped perf.

[xtqqczze]

It's probably ok to use DEFAULT_BUF_SIZE (8196 bytes) for the stack array, we already do so here:

coreutils/src/uu/tee/src/tee.rs

Line 230 in e85d07d

let mut buffer = [0u8; DEFAULT_BUF_SIZE];

[oech3]

It is too large. Would cause serious performance drop. Smaller size was faster.

[xtqqczze]

I guess there is overhead from zeroing. I'm interested to see your code for reusing BufReader (uses 8K internally), do you have a branch in your fork?

[oech3]

It seems I was misunderstanding (or perf is very flakey). It was just +4.16%

--- a/src/uu/unexpand/src/unexpand.rs
+++ b/src/uu/unexpand/src/unexpand.rs
@@ -8,7 +8,7 @@
 use clap::{Arg, ArgAction, Command};
 use std::ffi::OsString;
 use std::fs::File;
-use std::io::{BufReader, BufWriter, Read, Stdout, Write, stdin, stdout};
+use std::io::{BufRead, BufReader, BufWriter, Read, Stdout, Write, stdin, stdout};
 use std::num::IntErrorKind;
 use std::path::Path;
 use std::str::from_utf8;
@@ -553,7 +553,6 @@ fn unexpand_file(
     lastcol: usize,
     tab_config: &TabConfig,
 ) -> UResult<()> {
-    let mut buf = [0u8; 4096];
     let mut input = open(file)?;
     let mut print_state = PrintState {
         col: 0,
@@ -563,18 +562,21 @@ fn unexpand_file(
     };
 
     loop {
-        match input.read(&mut buf) {
-            Ok(0) => break,
-            Ok(n) => {
-                for line in buf[..n].split_inclusive(|b| *b == b'\n') {
-                    unexpand_buf(line, output, options, lastcol, tab_config, &mut print_state)?;
-                    if let Some(b'\n') = line.last() {
-                        print_state.new_line();
-                    }
-                }
+        let buf = input
+            .fill_buf()
+            .map_err_context(|| file.maybe_quote().to_string())?;
+        let buf_len = buf.len();
+        if buf_len == 0 {
+            break;
+        }
+        for line in buf.split_inclusive(|b| *b == b'\n') {
+            unexpand_buf(line, output, options, lastcol, tab_config, &mut print_state)?;
+            if let Some(b'\n') = line.last() {
+                print_state.new_line();
             }
-            Err(e) => return Err(e.map_err_context(|| file.maybe_quote().to_string())),
         }
+
+        input.consume(buf_len);
     }
     // write out anything remaining
     write_tabs(output, tab_config, &mut print_state, options.aflag)?;

[oech3]

I might try to avoid 0fill

[xtqqczze]

@oech3 I doubt there is much to be gained from avoiding zero fill to a stack allocated array. I think we could wait for the Rust standard library to gain support with Read::read_buf.

If you could remove reuse existing BufReader though, that would be great.

[oech3]

Should we try reusing BufReader with same buf size?

[oech3]

One of a possibility is unexpand was causing stack spill.
expand's perf was dropped with similar change.

[oech3]

still cgu=2 is 7% better