Skip to content

Commit

Permalink
Auto merge of rust-lang#134547 - SUPERCILEX:unify-copy, r=thomcc
Browse files Browse the repository at this point in the history
Unify fs::copy and io::copy on Linux

Currently, `fs::copy` first tries a regular file copy (via copy_file_range) and then falls back to userspace read/write copying. We should use `io::copy` instead as it tries copy_file_range, sendfile, and splice before falling back to userspace copying. This was discovered here: SUPERCILEX/fuc#40

Perf impact: `fs::copy` will now have two additional statx calls to decide which syscall to use. I wonder if we should get rid of the statx calls and only continue down the next fallback when the relevant syscalls say the FD isn't supported.
  • Loading branch information
bors committed Dec 28, 2024
2 parents 4e0bc49 + 96cc078 commit 3c1e750
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 24 deletions.
79 changes: 56 additions & 23 deletions library/std/src/sys/pal/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1948,7 +1948,7 @@ fn open_from(from: &Path) -> io::Result<(crate::fs::File, crate::fs::Metadata)>
#[cfg(target_os = "espidf")]
fn open_to_and_set_permissions(
to: &Path,
_reader_metadata: crate::fs::Metadata,
_reader_metadata: &crate::fs::Metadata,
) -> io::Result<(crate::fs::File, crate::fs::Metadata)> {
use crate::fs::OpenOptions;
let writer = OpenOptions::new().open(to)?;
Expand All @@ -1959,7 +1959,7 @@ fn open_to_and_set_permissions(
#[cfg(not(target_os = "espidf"))]
fn open_to_and_set_permissions(
to: &Path,
reader_metadata: crate::fs::Metadata,
reader_metadata: &crate::fs::Metadata,
) -> io::Result<(crate::fs::File, crate::fs::Metadata)> {
use crate::fs::OpenOptions;
use crate::os::unix::fs::{OpenOptionsExt, PermissionsExt};
Expand All @@ -1984,30 +1984,63 @@ fn open_to_and_set_permissions(
Ok((writer, writer_metadata))
}

#[cfg(not(any(target_os = "linux", target_os = "android", target_vendor = "apple")))]
pub fn copy(from: &Path, to: &Path) -> io::Result<u64> {
let (mut reader, reader_metadata) = open_from(from)?;
let (mut writer, _) = open_to_and_set_permissions(to, reader_metadata)?;
mod cfm {
use crate::fs::{File, Metadata};
use crate::io::{BorrowedCursor, IoSlice, IoSliceMut, Read, Result, Write};

io::copy(&mut reader, &mut writer)
}
#[allow(dead_code)]
pub struct CachedFileMetadata(pub File, pub Metadata);

impl Read for CachedFileMetadata {
fn read(&mut self, buf: &mut [u8]) -> Result<usize> {
self.0.read(buf)
}
fn read_vectored(&mut self, bufs: &mut [IoSliceMut<'_>]) -> Result<usize> {
self.0.read_vectored(bufs)
}
fn read_buf(&mut self, cursor: BorrowedCursor<'_>) -> Result<()> {
self.0.read_buf(cursor)
}
#[inline]
fn is_read_vectored(&self) -> bool {
self.0.is_read_vectored()
}
fn read_to_end(&mut self, buf: &mut Vec<u8>) -> Result<usize> {
self.0.read_to_end(buf)
}
fn read_to_string(&mut self, buf: &mut String) -> Result<usize> {
self.0.read_to_string(buf)
}
}
impl Write for CachedFileMetadata {
fn write(&mut self, buf: &[u8]) -> Result<usize> {
self.0.write(buf)
}
fn write_vectored(&mut self, bufs: &[IoSlice<'_>]) -> Result<usize> {
self.0.write_vectored(bufs)
}
#[inline]
fn is_write_vectored(&self) -> bool {
self.0.is_write_vectored()
}
#[inline]
fn flush(&mut self) -> Result<()> {
self.0.flush()
}
}
}
#[cfg(any(target_os = "linux", target_os = "android"))]
pub(crate) use cfm::CachedFileMetadata;

#[cfg(not(target_vendor = "apple"))]
pub fn copy(from: &Path, to: &Path) -> io::Result<u64> {
let (mut reader, reader_metadata) = open_from(from)?;
let max_len = u64::MAX;
let (mut writer, _) = open_to_and_set_permissions(to, reader_metadata)?;

use super::kernel_copy::{CopyResult, copy_regular_files};

match copy_regular_files(reader.as_raw_fd(), writer.as_raw_fd(), max_len) {
CopyResult::Ended(bytes) => Ok(bytes),
CopyResult::Error(e, _) => Err(e),
CopyResult::Fallback(written) => match io::copy::generic_copy(&mut reader, &mut writer) {
Ok(bytes) => Ok(bytes + written),
Err(e) => Err(e),
},
}
let (reader, reader_metadata) = open_from(from)?;
let (writer, writer_metadata) = open_to_and_set_permissions(to, &reader_metadata)?;

io::copy(
&mut cfm::CachedFileMetadata(reader, reader_metadata),
&mut cfm::CachedFileMetadata(writer, writer_metadata),
)
}

#[cfg(target_vendor = "apple")]
Expand Down Expand Up @@ -2044,7 +2077,7 @@ pub fn copy(from: &Path, to: &Path) -> io::Result<u64> {
}

// Fall back to using `fcopyfile` if `fclonefileat` does not succeed.
let (writer, writer_metadata) = open_to_and_set_permissions(to, reader_metadata)?;
let (writer, writer_metadata) = open_to_and_set_permissions(to, &reader_metadata)?;

// We ensure that `FreeOnDrop` never contains a null pointer so it is
// always safe to call `copyfile_state_free`
Expand Down
15 changes: 14 additions & 1 deletion library/std/src/sys/pal/unix/kernel_copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ use crate::process::{ChildStderr, ChildStdin, ChildStdout};
use crate::ptr;
use crate::sync::atomic::{AtomicBool, AtomicU8, Ordering};
use crate::sys::cvt;
use crate::sys::fs::CachedFileMetadata;
use crate::sys::weak::syscall;

#[cfg(test)]
Expand Down Expand Up @@ -192,7 +193,7 @@ impl<R: CopyRead, W: CopyWrite> SpecCopy for Copier<'_, '_, R, W> {
let w_cfg = writer.properties();

// before direct operations on file descriptors ensure that all source and sink buffers are empty
let mut flush = || -> crate::io::Result<u64> {
let mut flush = || -> Result<u64> {
let bytes = reader.drain_to(writer, u64::MAX)?;
// BufWriter buffered bytes have already been accounted for in earlier write() calls
writer.flush()?;
Expand Down Expand Up @@ -537,6 +538,18 @@ impl<T: ?Sized + CopyWrite> CopyWrite for BufWriter<T> {
}
}

impl CopyRead for CachedFileMetadata {
fn properties(&self) -> CopyParams {
CopyParams(FdMeta::Metadata(self.1.clone()), Some(self.0.as_raw_fd()))
}
}

impl CopyWrite for CachedFileMetadata {
fn properties(&self) -> CopyParams {
CopyParams(FdMeta::Metadata(self.1.clone()), Some(self.0.as_raw_fd()))
}
}

fn fd_to_meta<T: AsRawFd>(fd: &T) -> FdMeta {
let fd = fd.as_raw_fd();
let file: ManuallyDrop<File> = ManuallyDrop::new(unsafe { File::from_raw_fd(fd) });
Expand Down

0 comments on commit 3c1e750

Please sign in to comment.