diff --git a/crates/librqbit/src/storage/filesystem/fs.rs b/crates/librqbit/src/storage/filesystem/fs.rs index 38a38929c..a28055f36 100644 --- a/crates/librqbit/src/storage/filesystem/fs.rs +++ b/crates/librqbit/src/storage/filesystem/fs.rs @@ -1,5 +1,4 @@ use std::{ - fs::OpenOptions, io::IoSlice, path::{Path, PathBuf}, }; @@ -14,10 +13,26 @@ use crate::{ use crate::storage::{StorageFactory, TorrentStorage}; -use super::opened_file::OpenedFile; +use super::opened_file::{open_file, OpenedFile}; #[derive(Default, Clone, Copy)] -pub struct FilesystemStorageFactory {} +pub struct FilesystemStorageFactory { + lazy_open: bool, +} + +impl FilesystemStorageFactory { + /// Open backing files on first access instead of all up front at `init`. + /// + /// Eager opening makes restoring many/large torrents slow: `init` runs on + /// the add path and opens every file of every torrent before the session is + /// usable. With lazy opening, files open on demand (during transfer), and + /// `ensure_file_length` skips files already at the correct length — so + /// restoring already-complete torrents opens nothing. + pub fn with_lazy_open(mut self, lazy_open: bool) -> Self { + self.lazy_open = lazy_open; + self + } +} impl StorageFactory for FilesystemStorageFactory { type Storage = FilesystemStorage; @@ -30,6 +45,7 @@ impl StorageFactory for FilesystemStorageFactory { Ok(FilesystemStorage { output_folder: shared.options.output_folder.clone(), opened_files: Default::default(), + lazy_open: self.lazy_open, }) } @@ -41,6 +57,7 @@ impl StorageFactory for FilesystemStorageFactory { pub struct FilesystemStorage { pub(crate) output_folder: PathBuf, pub(crate) opened_files: Vec, + pub(crate) lazy_open: bool, } impl FilesystemStorage { @@ -53,6 +70,7 @@ impl FilesystemStorage { .map(|f| f.take_clone()) .collect::>>()?, output_folder: self.output_folder.clone(), + lazy_open: self.lazy_open, }) } } @@ -93,6 +111,17 @@ impl TorrentStorage for FilesystemStorage { fn ensure_file_length(&self, file_id: usize, len: u64) -> anyhow::Result<()> { let f = &self.opened_files.get(file_id).context("no such file")?; + // Lazy fast path: if the file already exists at the target length, don't + // open it just to set_len. This lets restoring a complete torrent open + // none of its files. + let already_correct = f + .lazy_unopened_path() + .and_then(|path| std::fs::metadata(path).ok()) + .map(|m| m.len() == len) + .unwrap_or(false); + if already_correct { + return Ok(()); + } #[cfg(windows)] f.try_mark_sparse()?; Ok(f.lock_read()?.set_len(len)?) @@ -106,6 +135,7 @@ impl TorrentStorage for FilesystemStorage { .map(|f| f.take_clone()) .collect::>>()?, output_folder: self.output_folder.clone(), + lazy_open: self.lazy_open, })) } @@ -137,33 +167,91 @@ impl TorrentStorage for FilesystemStorage { files.push(OpenedFile::new_dummy()); continue; }; - std::fs::create_dir_all(full_path.parent().context("bug: no parent")?)?; - let f = if shared.options.allow_overwrite { - OpenOptions::new() - .create(true) - .truncate(false) - .read(true) - .write(true) - .open(&full_path) - .with_context(|| format!("error opening {full_path:?} in read/write mode"))? + if self.lazy_open { + // Defer create_dir_all + open to first access + // (see FilesystemStorageFactory::with_lazy_open). + files.push(OpenedFile::new_lazy( + full_path, + shared.options.allow_overwrite, + )); } else { - // create_new does not seem to work with read(true), so calling this twice. - OpenOptions::new() - .create_new(true) - .write(true) - .open(&full_path) - .with_context(|| { - format!( - "error creating a new file (because allow_overwrite = false) {:?}", - &full_path - ) - })?; - OpenOptions::new().read(true).write(true).open(&full_path)? - }; - files.push(OpenedFile::new(full_path.clone(), f)); + let f = open_file(&full_path, shared.options.allow_overwrite)?; + files.push(OpenedFile::new(full_path, f)); + } } self.opened_files = files; Ok(()) } } + +#[cfg(test)] +mod lazy_tests { + use super::*; + + fn lazy_storage(files: Vec, folder: PathBuf) -> FilesystemStorage { + FilesystemStorage { + output_folder: folder, + opened_files: files, + lazy_open: true, + } + } + + #[test] + fn lazy_reads_existing_file_only_on_first_access() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("a.bin"); + std::fs::write(&path, b"hello world").unwrap(); + + let of = OpenedFile::new_lazy(path, true); + assert!( + of.lazy_unopened_path().is_some(), + "must not open until accessed" + ); + let s = lazy_storage(vec![of], dir.path().to_path_buf()); + + let mut buf = [0u8; 5]; + s.pread_exact(0, 6, &mut buf).unwrap(); + assert_eq!(&buf, b"world"); + assert!( + s.opened_files[0].lazy_unopened_path().is_none(), + "pread should have opened it" + ); + } + + #[test] + fn ensure_file_length_is_stat_first() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("b.bin"); + std::fs::write(&path, b"1234567890").unwrap(); // len 10 + + let s = lazy_storage( + vec![OpenedFile::new_lazy(path.clone(), true)], + dir.path().to_path_buf(), + ); + + s.ensure_file_length(0, 10).unwrap(); + assert!( + s.opened_files[0].lazy_unopened_path().is_some(), + "correct length must not open the file" + ); + assert_eq!(std::fs::read(&path).unwrap(), b"1234567890"); + + s.ensure_file_length(0, 4).unwrap(); + assert_eq!(std::fs::metadata(&path).unwrap().len(), 4); + } + + #[test] + fn lazy_pwrite_creates_file_and_parent_dir() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("nested/c.bin"); + let s = lazy_storage( + vec![OpenedFile::new_lazy(path.clone(), true)], + dir.path().to_path_buf(), + ); + + s.ensure_file_length(0, 4).unwrap(); + s.pwrite_all(0, 0, b"abcd").unwrap(); + assert_eq!(std::fs::read(&path).unwrap(), b"abcd"); + } +} diff --git a/crates/librqbit/src/storage/filesystem/opened_file.rs b/crates/librqbit/src/storage/filesystem/opened_file.rs index a0aed8dc9..e82ea5df6 100644 --- a/crates/librqbit/src/storage/filesystem/opened_file.rs +++ b/crates/librqbit/src/storage/filesystem/opened_file.rs @@ -1,8 +1,8 @@ use std::{ - fs::File, + fs::{File, OpenOptions}, io::IoSlice, ops::{Deref, DerefMut}, - path::PathBuf, + path::{Path, PathBuf}, }; use anyhow::Context; @@ -101,9 +101,12 @@ impl OurFileExt for File { #[derive(Default, Debug)] struct OpenedFileLocked { - #[allow(unused)] path: PathBuf, fd: Option, + // When `lazy`, `fd` is opened on first access (using `path` + `overwrite`) + // instead of up front at init. + lazy: bool, + overwrite: bool, #[cfg(windows)] tried_marking_sparse: bool, } @@ -133,6 +136,8 @@ impl OpenedFile { file: RwLock::new(OpenedFileLocked { path, fd: Some(f), + lazy: false, + overwrite: false, #[cfg(windows)] tried_marking_sparse: false, }), @@ -145,6 +150,45 @@ impl OpenedFile { } } + /// A real file opened on first access instead of up front at init. + pub fn new_lazy(path: PathBuf, overwrite: bool) -> Self { + Self { + file: RwLock::new(OpenedFileLocked { + path, + fd: None, + lazy: true, + overwrite, + #[cfg(windows)] + tried_marking_sparse: false, + }), + } + } + + /// Open the backing file if it isn't open yet. A no-op for eager or padding + /// handles and for already-open files. The open happens once, under the + /// write lock. + fn ensure_open(&self) -> crate::Result<()> { + { + let g = self.file.read(); + if g.fd.is_some() || !g.lazy { + return Ok(()); + } + } + let mut g = self.file.write(); + if g.fd.is_none() && g.lazy { + let f = open_file(&g.path, g.overwrite).map_err(Error::Anyhow)?; + g.fd = Some(f); + } + Ok(()) + } + + /// The path of a lazily-openable file not yet opened (for stat-first + /// checks). `None` once opened, or for eager/padding handles. + pub fn lazy_unopened_path(&self) -> Option { + let g = self.file.read(); + (g.lazy && g.fd.is_none()).then(|| g.path.clone()) + } + pub fn take_clone(&self) -> anyhow::Result { let f = std::mem::take(&mut *self.file.write()); Ok(Self { @@ -153,6 +197,7 @@ impl OpenedFile { } pub fn lock_read(&self) -> crate::Result> { + self.ensure_open()?; RwLockReadGuard::try_map(self.file.read(), |f| f.as_ref()) .ok() .ok_or(Error::FsFileIsNone) @@ -160,6 +205,7 @@ impl OpenedFile { #[allow(dead_code)] pub fn lock_write(&self) -> crate::Result> { + self.ensure_open()?; RwLockWriteGuard::try_map(self.file.write(), |f| f.as_mut()) .ok() .ok_or(Error::FsFileIsNone) @@ -167,6 +213,7 @@ impl OpenedFile { #[cfg(windows)] pub fn try_mark_sparse(&self) -> crate::Result> { + self.ensure_open()?; { let g = self.file.read(); if g.tried_marking_sparse { @@ -186,6 +233,35 @@ impl OpenedFile { } } +/// Open (creating dirs/file as needed) a torrent file for read+write. Shared by +/// eager `init` and lazy `ensure_open` so both use identical semantics. +pub(super) fn open_file(full_path: &Path, allow_overwrite: bool) -> anyhow::Result { + std::fs::create_dir_all(full_path.parent().context("bug: no parent")?)?; + let f = if allow_overwrite { + OpenOptions::new() + .create(true) + .truncate(false) + .read(true) + .write(true) + .open(full_path) + .with_context(|| format!("error opening {full_path:?} in read/write mode"))? + } else { + // create_new does not seem to work with read(true), so calling this twice. + OpenOptions::new() + .create_new(true) + .write(true) + .open(full_path) + .with_context(|| { + format!( + "error creating a new file (because allow_overwrite = false) {:?}", + full_path + ) + })?; + OpenOptions::new().read(true).write(true).open(full_path)? + }; + Ok(f) +} + #[cfg(test)] mod tests { use std::io::Read;