Use splice syscall on Linux to greatly improve cat performance#1289
Use splice syscall on Linux to greatly improve cat performance#1289ArniDagur wants to merge 6 commits intouutils:masterfrom
splice syscall on Linux to greatly improve cat performance#1289Conversation
|
Based on what I'm seeing in that thread, I'm imagining it might be due to some kernel setting (or combination of kernel settings). Have you tested using different shells? I'm not using Linux atm, so I can't check myself until later this week. |
|
@Arcterus It turns out that the problem lied in my use of the following two lines in my I intend to file an issue with the keychain people soon, as this could be a preventable bug. Fastcat and my not-so-great tee clone now work as intended, and on the topic of this pull request, a quick test yields positive results: Source branch (this pull request): [arni: coreutils (use-splice)]$ target/release/cat /tmp/largefile | pv -r > /dev/null
[10.8GiB/s]
[arni: coreutils (use-splice)]$ target/release/cat /tmp/largefile | pv -r > /dev/null
[11.2GiB/s]
[arni: coreutils (use-splice)]$ target/release/cat /tmp/largefile | pv -r > /dev/null
[11.0GiB/s]Master: [arni: coreutils (master)]$ target/release/cat /tmp/largefile | pv -r > /dev/null
[3.94GiB/s]
[arni: coreutils (master)]$ target/release/cat /tmp/largefile | pv -r > /dev/null
[3.96GiB/s]
[arni: coreutils (master)]$ target/release/cat /tmp/largefile | pv -r > /dev/null
[3.87GiB/s]Please follow up later this week when you get back to your Linux machine :) |
|
@Arcterus The unit tests pass for me. Do you mind taking a look at this PR? |
ArniDagur
changed the title
splice syscall on Linux to greatly improve cat performancesplice syscall on Linux to greatly improve cat performance
Arcterus
left a comment
Arcterus
left a comment
There was a problem hiding this comment.
I think this is good. It might be nice to add a feature letting the user just use the standard behavior on Linux as well (it's not necessary though).
|
@Arcterus Does this PR look better to you now? |
Arcterus
left a comment
Arcterus
left a comment
There was a problem hiding this comment.
If you have time, I'd be interested in adding a reader/writer like this into uucore.
* Use `io::copy` as `write_fast` fallback. * Document `write_fast_using_splice` with Rustdoc comment. * Inline `write_fast_using_splice`. * Ignore all `splice` related errors.
src/cat/cat.rs
Outdated
| }, | ||
| // If we're not on Linux or Android, or the splice() call failed, | ||
| // fall back on slower writing. | ||
| io::copy(&mut handle.reader, &mut writer_handle).unwrap(); |
There was a problem hiding this comment.
The old implementation was actually incorrect in that this should not panic/cause an error. It should write out the error message and then bump the error count like in the Err(error) clause below.
Arcterus
left a comment
Arcterus
left a comment
There was a problem hiding this comment.
Sorry for the late response! If you don't want to make these changes, at least make issues for them so we can keep track.
| @@ -456,7 +518,8 @@ fn write_nonprint_to_end<W: Write>(in_buf: &[u8], writer: &mut W, tab: &[u8]) -> | |||
| 128...159 => writer.write_all(&[b'M', b'-', b'^', byte - 64]), | |||
There was a problem hiding this comment.
Can you change this to use byte literals (and maybe fix the other issues mentioned by the person on IRC)?
| return Ok(InputHandle { | ||
| #[cfg(any(target_os = "linux", target_os = "android"))] | ||
| file_descriptor: stdin.as_raw_fd(), | ||
| reader: Box::new(stdin) as Box<Read>, |
There was a problem hiding this comment.
I mentioned this on the other PR as well, but could you change this to avoid Box?
|
For the specific case of catting from regular file(s) to a regular file one could also try |
rivy
force-pushed
the
master
branch
4 times, most recently
from
f07992e to
813e57d
Compare
|
Note that with rust 1.50 onwards the standard library's You'll probably get the same benefit by simply switching to |
|
@ArniDagur As wc landed, do you want to finish this one too ? :) |
|
New PR: #1978 |
4 participants
This is what mre/fcat does to great effect (3x faster than GNU cat). Unfortunately, when testing the source branch, I get the same error as described in mre/fcat#12, so there are definitely bugs that need to be dealt with. Thus I labelled this PR work in progress.
If someone here could test the source branch on their machine to see if they get the same error, do code review, or give other feedback I would very much appreciate it :)