Add opt-in FilesystemStorageFactory::with_lazy_open (open files on first access) - #629
Open
ikaradimas wants to merge 1 commit into
Open
Add opt-in FilesystemStorageFactory::with_lazy_open (open files on first access)#629ikaradimas wants to merge 1 commit into
ikaradimas wants to merge 1 commit into
Conversation
…pen files on first access) FilesystemStorage::init opens every file of a torrent up front, and init runs on the add path (via create_and_init in Session::add_torrent) - so restoring a session opens every file of every torrent before the session is usable. For a large, many-file library that is thousands of open() calls (~87s to restore 41 complete torrents on Windows in one report), even when all data is already present and no hashing is needed. Add an opt-in FilesystemStorageFactory::with_lazy_open (default false). When enabled, init records each file's path instead of opening it; the backing file is opened on first pread/pwrite/ensure_file_length (guarded so it happens once). ensure_file_length is also stat-first: a file already at the target length is neither opened nor resized. A restored complete torrent therefore opens none of its files at startup - they open on demand during transfer. MmapFilesystemStorage builds its inner FilesystemStorage via FilesystemStorageFactory::default() and needs the files open after init to mmap them, so it keeps eager opening (flag off) and is unaffected. The read/write paths are unchanged apart from the lazy-open guard. Unit tests cover lazy read of an existing file, stat-first ensure_file_length, and lazy write (creates file + parent dir).
Owner
|
Will this do anything at all without your other "trust_fastresume" change? As each file either needs to be checked (at least to a degree, with fastresume), or written to, this would just move the opening to a later stage (during download). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
FilesystemStorage::initopens every file of a torrent up front, and init runs on the add path (viacreate_and_initinSession::add_torrent) — so restoring a session opens every file of every torrent before the session is usable. For a large, many-file library that is thousands of open() calls: on one user's Windows session, ~87s to restore 41 torrents even with all data already complete and no hashing (see also thetrust_fastresumePR). Files are only actually read/written on demand during transfer, so opening them all eagerly is wasted work at the worst possible time.Change
Add an opt-in
FilesystemStorageFactory::with_lazy_open(bool)(defaultfalse, so existing behavior is unchanged). When enabled, init records each file's path instead of opening it; the backing file is opened on firstpread/pwrite/ensure_file_length(guarded so it happens once). In addition,ensure_file_lengthbecomes stat-first: a file already at the target length is neither opened nor resized. A restored complete torrent therefore opens none of its files at startup — they open on demand.MmapFilesystemStoragebuilds its innerFilesystemStorageviaFilesystemStorageFactory::default()and needs the files open after init to mmap them, so it keeps eager opening (flag off) and is unaffected. The read and write paths are unchanged apart from the lazy-open guard.Testing
Unit tests in
storage/filesystem/fs.rscover lazy read of an existing file (opens only on first access, returns correct bytes), stat-firstensure_file_length(no open when already the right length), and lazy write (creates the file and parent dir). With the flag false (default), behavior is unchanged.