Repository navigation
yes: separate platform-specific output implementations - #15478
Conversation
| // it's possible that using a smaller or larger buffer might provide better performance | ||
| pub(super) const BUF_SIZE: usize = 16 * 1024; |
There was a problem hiding this comment.
should we use st_blksize on stdout ? Otherwise, after some tests I suggest 64*1024 for performance
There was a problem hiding this comment.
I think this is platform specific pipe size. Someone said Windows's pipe is very small.
There was a problem hiding this comment.
But yes > file should use larger size.
There was a problem hiding this comment.
(Don't you use yes | pv > /dev/null on Linux or Android if you tune this)
There was a problem hiding this comment.
If we aren't on a pipe maybe the extra syscall to get the blksize is worth it ? In any case, maybe the pipe size should be a platform specific const ?
There was a problem hiding this comment.
If we use yes for pipe, we need 1st few byte to tell y to script at most cases.
Otherwise yes is used for generating large test data or wiping strage device quickly.
So we could send 1st (page size) and switch to 64 KiB later as fast-path.
But this PR should not improve perf.
| // it's possible that using a smaller or larger buffer might provide better performance | ||
| #[cfg(not(any(target_os = "linux", target_os = "android")))] | ||
| const BUF_SIZE: usize = 16 * 1024; | ||
| #[cfg_attr(any(target_os = "linux", target_os = "android"), path = "linux.rs")] |
There was a problem hiding this comment.
could you please move them to src/platform/ like the other utils? and two plain #[cfg] #[path] lines like nohup would be easier to read than cfg_attr
3885bc7 to
ce5bac8
Compare
| use std::ffi::OsString; | ||
| use std::io::{self, Write}; | ||
| #[cfg(any(windows, target_os = "wasi"))] | ||
| use std::io; |
There was a problem hiding this comment.
could you please drop this cfg'd import and use std::io::ErrorKind::BrokenPipe directly below?
Move the Linux/Android output loop and buffer size into linux.rs, and the portable writer into portable.rs. Keep command parsing and buffer preparation in yes.rs and re-export the existing exec entry point. Validation: 10 integration tests and 1 unit test pass; formatting, Clippy, and macOS/FreeBSD compilation checks pass. Closes uutils#12371
ce5bac8 to
e295ef3
Compare
|
|
Thanks for your PR |
4 participants
Move the Linux/Android output loop and buffer size into linux.rs, and the portable writer into portable.rs. Keep command parsing and buffer preparation in yes.rs and re-export the existing exec entry point.
Validation: 10 integration tests and 1 unit test pass; formatting, Clippy, and macOS/FreeBSD compilation checks pass.
Closes #12371