Add opt-in FilesystemStorageFactory::with_lazy_open (open files on first access)

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<OpenedFile>,
+    pub(crate) lazy_open: bool,
 }
 
 impl FilesystemStorage {
@@ -53,6 +70,7 @@ impl FilesystemStorage {
                 .map(|f| f.take_clone())
                 .collect::<anyhow::Result<Vec<_>>>()?,
             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::<anyhow::Result<Vec<_>>>()?,
             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<OpenedFile>, 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");
+    }
+}

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<File>,
+    // 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<PathBuf> {
+        let g = self.file.read();
+        (g.lazy && g.fd.is_none()).then(|| g.path.clone())
+    }
+
     pub fn take_clone(&self) -> anyhow::Result<Self> {
         let f = std::mem::take(&mut *self.file.write());
         Ok(Self {
@@ -153,20 +197,23 @@ impl OpenedFile {
     }
 
     pub fn lock_read(&self) -> crate::Result<impl Deref<Target = File>> {
+        self.ensure_open()?;
         RwLockReadGuard::try_map(self.file.read(), |f| f.as_ref())
             .ok()
             .ok_or(Error::FsFileIsNone)
     }
 
     #[allow(dead_code)]
     pub fn lock_write(&self) -> crate::Result<impl DerefMut<Target = File>> {
+        self.ensure_open()?;
         RwLockWriteGuard::try_map(self.file.write(), |f| f.as_mut())
             .ok()
             .ok_or(Error::FsFileIsNone)
     }
 
     #[cfg(windows)]
     pub fn try_mark_sparse(&self) -> crate::Result<impl Deref<Target = File>> {
+        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<File> {
+    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;