fast_io.rs: use sendfile or splice if copy_file_range is not supported - #459
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #459 +/- ##
==========================================
- Coverage 83.27% 83.17% -0.10%
==========================================
Files 13 13
Lines 6994 7119 +125
Branches 397 401 +4
==========================================
+ Hits 5824 5921 +97
- Misses 1167 1194 +27
- Partials 3 4 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
cee18ce to
581e725
Compare
|
sendfile is internally splice. So no worth to try it. splice with middler pipe is faster. |
|
Please use rustix instead of unsafe libc. |
|
@Franklin-Qi sorry, it needs to be rebased |
581e725 to
bd5b0e2
Compare
Merging this PR will degrade performance by 0.97%
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing 
|
6bb25e7 to
7551756
Compare
- Use `splice` and attempt to expand the pipe size when necessary. - Modify the `reliable_copy_file_range()` function to sequentially attempt `copy_file_range` → `splice` → `sendfile` → `write`. - Add Linux-based unit tests. Closes: uutils#443
7551756 to
1889db5
Compare
| struct FastCopy { | ||
| fd: i32, // Raw file descriptor | ||
| is_regular: bool, // True if this is a regular file | ||
| is_fifo: bool, // True if this is a pipe/FIFO (splice-capable) |
There was a problem hiding this comment.
We don't need to check that input or output is pipe if middler pipe is used. Also splice fails if both of in/output are not pipe. So that bool should be unnecessary.
| #[cfg(target_os = "linux")] | ||
| use rustix::fs::copy_file_range as rustix_copy_file_range; | ||
|
|
||
| #[cfg(all(target_os = "linux", target_env = "gnu"))] |
There was a problem hiding this comment.
| #[cfg(all(target_os = "linux", target_env = "gnu"))] | |
| #[cfg(target_os = "linux")] |
Same for others.
|
|
||
| /// Best-effort pipe capacity bump. Larger size is optional, so failures are ignored. | ||
| #[cfg(all(target_os = "linux", target_env = "gnu"))] | ||
| fn set_pipe_size(fd: impl AsFd, size: usize) -> Result<(), ()> { |
There was a problem hiding this comment.
Why do we need to wrap this even we just ignore the error?
| out_fd: BorrowedFd<'_>, | ||
| len: usize, | ||
| ) -> std::io::Result<Option<usize>> { | ||
| match splice(in_fd, Some(in_off), out_fd, None, len, SpliceFlags::MORE) { |
There was a problem hiding this comment.
I don't think MORE is useful at here.
| /// Falls back to write(2) if splice is not supported. | ||
| #[cfg(all(target_os = "linux", target_env = "gnu"))] | ||
| fn reliable_splice( | ||
| in_ptr: *const u8, |
There was a problem hiding this comment.
Why is this needed (with unsafe code)?
| // Spawn a reader that drains slowly | ||
| thread::spawn(move || { | ||
| let mut buf = [0u8; 1024]; | ||
| loop { |
There was a problem hiding this comment.
while let Ok(1..) = reader.read(&mut buf) {...}
3 participants
splice(via rustix) and attempt to expand the pipe size when necessary.reliable_copy_file_range()to trycopy_file_range→splice→write.Closes: #443