Skip to content

Commit 5469661

Browse files
committed
refactor: remove dedicated splice code path
It is now done transparently by the Rust stdlib: rust-lang/rust#75272
1 parent 837c2f7 commit 5469661

File tree

5 files changed

+12
-107
lines changed

5 files changed

+12
-107
lines changed

.pre-commit-config.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,4 +48,4 @@ repos:
4848
hooks:
4949
- id: conventional-pre-commit
5050
stages: [commit-msg]
51-
args: [chore, config, doc, test]
51+
args: [chore, config, doc, refactor, test]

Cargo.lock

Lines changed: 0 additions & 29 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,3 @@ toml = { version = "0.5.9", default-features = false }
2727
tree_magic_mini = { version = "3.0.3", default-features = false }
2828
url = { version = "2.3.1", default-features = false }
2929
xdg = { version = "2.4.1", default-features = false }
30-
31-
[target.'cfg(any(target_os = "linux", target_os = "android"))'.dependencies]
32-
nix = { version = "0.22.3", default-features = false }

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ pa() {
125125
## Performance
126126

127127
`rsop` is quite fast. In practice it rarely matters because choosing with which program to open or preview files is usually so quick it is not perceptible. However performance can matter if for example you are decompressing a huge `tar.gz` archive to preview its content.
128-
To help with that, `rsop` uses the [`splice` system call](https://man7.org/linux/man-pages/man2/splice.2.html) if available on your platform. In the `.tar.gz` example this allows decompressing data with `gzip` or `pigz` and passing it to tar (or whatever you have configured to handle `application/x-tar` MIME type), **without wasting time to copy data in user space** between the two programs.
128+
To help with that, `rsop` uses the [`splice` system call](https://man7.org/linux/man-pages/man2/splice.2.html) if available on your platform. In the `.tar.gz` example this allows decompressing data with `gzip` or `pigz` and passing it to tar (or whatever you have configured to handle `application/x-tar` MIME type), **without wasting time to copy data in user space** between the two programs. This was previously done using a custom code path, but is now done [transparently](https://github.com/rust-lang/rust/pull/75272) by the standard library.
129129

130130
Other stuff `rsop` does to remain quick:
131131

src/handler.rs

Lines changed: 10 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,12 @@
11
use std::collections::HashMap;
22
use std::env;
33
use std::fs::File;
4-
#[cfg(not(any(target_os = "linux", target_os = "android")))]
5-
use std::io::copy;
6-
use std::io::{self, stdin, Read, Write};
4+
use std::io::{self, copy, stdin, Read, Write};
75
use std::iter;
86
use std::os::unix::fs::FileTypeExt;
97
use std::os::unix::io::{AsRawFd, FromRawFd};
108
use std::path::{Path, PathBuf};
11-
use std::process::{Child, ChildStdout, Command, Stdio};
9+
use std::process::{Child, Command, Stdio};
1210
use std::rc::Rc;
1311

1412
use crate::config;
@@ -102,26 +100,8 @@ pub enum HandlerError {
102100
Other(#[from] anyhow::Error),
103101
}
104102

105-
#[cfg(any(target_os = "linux", target_os = "android"))]
106-
lazy_static::lazy_static! {
107-
// How many bytes to read from pipe to guess MIME type, use a full memory page
108-
static ref PIPE_INITIAL_READ_LENGTH: usize =
109-
nix::unistd::sysconf(nix::unistd::SysconfVar::PAGE_SIZE).expect("Unable to get page size").unwrap() as usize;
110-
}
111-
#[cfg(not(any(target_os = "linux", target_os = "android")))]
112-
lazy_static::lazy_static! {
113-
static ref PIPE_INITIAL_READ_LENGTH: usize = 4096;
114-
}
115-
116-
#[cfg(not(any(target_os = "linux", target_os = "android")))]
117-
trait ReadPipe: Read + Send {}
118-
119-
#[cfg(any(target_os = "linux", target_os = "android"))]
120-
trait ReadPipe: Read + AsRawFd + Send {}
121-
122-
impl ReadPipe for File {}
123-
124-
impl ReadPipe for ChildStdout {}
103+
/// How many bytes to read from pipe to guess MIME type, use a full memory page
104+
const PIPE_INITIAL_READ_LENGTH: usize = 4096;
125105

126106
impl HandlerMapping {
127107
pub fn new(cfg: &config::Config) -> anyhow::Result<HandlerMapping> {
@@ -312,7 +292,7 @@ impl HandlerMapping {
312292
#[allow(clippy::wildcard_in_or_patterns)]
313293
fn dispatch_pipe<T>(&self, mut pipe: T, mode: &RsopMode) -> Result<(), HandlerError>
314294
where
315-
T: ReadPipe,
295+
T: Read + Send,
316296
{
317297
// Handler candidates
318298
let (handlers, next_handlers) = match mode {
@@ -326,9 +306,9 @@ impl HandlerMapping {
326306
// Read header
327307
log::trace!(
328308
"Using max header length of {} bytes",
329-
*PIPE_INITIAL_READ_LENGTH
309+
PIPE_INITIAL_READ_LENGTH
330310
);
331-
let mut buffer: Vec<u8> = vec![0; *PIPE_INITIAL_READ_LENGTH];
311+
let mut buffer: Vec<u8> = vec![0; PIPE_INITIAL_READ_LENGTH];
332312
let header_len = pipe.read(&mut buffer)?;
333313
let header = &buffer[0..header_len];
334314

@@ -532,7 +512,7 @@ impl HandlerMapping {
532512
mode: &RsopMode,
533513
) -> Result<(), HandlerError>
534514
where
535-
T: ReadPipe,
515+
T: Read + Send,
536516
{
537517
let term_size = Self::term_size();
538518

@@ -623,7 +603,7 @@ impl HandlerMapping {
623603
term_size: &termsize::Size,
624604
) -> Result<(), HandlerError>
625605
where
626-
T: ReadPipe,
606+
T: Read,
627607
{
628608
// Write to a temporary file if handler does not support reading from stdin
629609
let input = if handler.no_pipe {
@@ -703,14 +683,11 @@ impl HandlerMapping {
703683
// Unfortunately, stdin is buffered, and there is no clean way to get it
704684
// unbuffered to read only what we want for the header, so use fd hack to get an unbuffered reader
705685
// see https://users.rust-lang.org/t/add-unbuffered-rawstdin-rawstdout/26013
706-
// On plaforms other than linux we don't care about buffering because we use chunk copy instead of splice
707686
let stdin = stdin();
708687
let reader = unsafe { File::from_raw_fd(stdin.as_raw_fd()) };
709688
Ok(reader)
710689
}
711690

712-
// Default chunk copy using stdlib's std::io::copy when splice syscall is not available
713-
#[cfg(not(any(target_os = "linux", target_os = "android")))]
714691
fn pipe_forward<S, D>(src: &mut S, dst: &mut D, header: &[u8]) -> anyhow::Result<usize>
715692
where
716693
S: Read,
@@ -728,49 +705,9 @@ impl HandlerMapping {
728705
Ok(header.len() + copied)
729706
}
730707

731-
// Efficient 0-copy implementation using splice
732-
#[cfg(any(target_os = "linux", target_os = "android"))]
733-
fn pipe_forward<S, D>(src: &mut S, dst: &mut D, header: &[u8]) -> anyhow::Result<usize>
734-
where
735-
S: AsRawFd,
736-
D: AsRawFd + Write,
737-
{
738-
dst.write_all(header)?;
739-
log::trace!("Header written ({} bytes)", header.len());
740-
741-
let mut c = 0;
742-
const SPLICE_LEN: usize = 2usize.pow(62); // splice returns -EINVAL for pipe to file with usize::MAX len
743-
const SPLICE_FLAGS: nix::fcntl::SpliceFFlags = nix::fcntl::SpliceFFlags::empty();
744-
745-
loop {
746-
let rc = nix::fcntl::splice(
747-
src.as_raw_fd(),
748-
None,
749-
dst.as_raw_fd(),
750-
None,
751-
SPLICE_LEN,
752-
SPLICE_FLAGS,
753-
);
754-
let moved = match rc {
755-
Err(nix::errno::Errno::EPIPE) => 0,
756-
Err(e) => return Err(anyhow::Error::new(e)),
757-
Ok(m) => m,
758-
};
759-
log::trace!("moved = {}", moved);
760-
if moved == 0 {
761-
break;
762-
}
763-
c += moved;
764-
}
765-
766-
log::trace!("Pipe exhausted, moved {} bytes total", header.len() + c);
767-
768-
Ok(header.len() + c)
769-
}
770-
771708
fn pipe_to_tmpfile<T>(header: &[u8], mut pipe: T) -> anyhow::Result<tempfile::NamedTempFile>
772709
where
773-
T: ReadPipe,
710+
T: Read,
774711
{
775712
let mut tmp_file = tempfile::Builder::new()
776713
.prefix(const_format::concatcp!(env!("CARGO_PKG_NAME"), '_'))

0 commit comments

Comments
 (0)