Stop stat-ing every entry when building the gallery - #48
Merged
Conversation
Gallery::new called Path::is_file() on every directory entry, which is a stat syscall, and it ran before the extension test because && evaluates left to right. Opening any file therefore stat'd the whole folder, images and unrelated files alike, on the path between the click and the first pixel. DirEntry::file_type() answers the same question from the d_type field that readdir already returned, with no syscall at all on Linux. That single swap is where the time goes. Measured on two 50,000 file directories: 86.6 ms to 18.0 ms for a folder holding 5,000 images among 45,000 other files, and 104.3 ms to 22.3 ms for one holding 50,000 images. The second case is the proof it is the syscall and not the filter, since the extension test rejects nothing there and it still gets 4.7x faster. The extension test now runs first anyway, so a rejected name never builds a PathBuf. Only survivors allocate. is_file() follows symlinks and file_type() does not, so the naive swap silently drops symlinked images. That is the whole reason for the is_symlink() branch: it falls back to the stat for those entries only, which keeps the old semantics at a cost nothing outside a folder of symlinks will ever notice. On filesystems that report DT_UNKNOWN, file_type() falls back to an lstat internally, so the worst case is what this code already did. gallery.rs had no tests. It now pins the filtering rules, including that extensions match case-insensitively, that a directory named like an image is excluded, that a dangling symlink is excluded, and that opening a symlinked image lands the gallery on that symlink rather than falling back to index 0 and reporting the wrong position. Verified discriminating: removing the is_symlink() fallback fails both symlink tests, listing ["real.jpg"] where ["linked.jpg", "real.jpg"] is expected, while the plain filtering test stays green because it does not depend on that branch. Two things measured and rejected. A HashSet lookup keyed on the extension lost to the linear scan in the all-images case, 17.3 ms against 16.2 ms, because jpg is the first entry in SUPPORTED. Sorting SUPPORTED to allow a binary search is not available either, since tasks.rs hands that same slice to the file dialog as its filter list and the order is user visible. The sort_unstable over 50,000 paths costs 3.6 ms and is not worth touching.
cargo fmt --check runs ahead of clippy in CI and rejected two hand-written lines in the new test module. No behaviour change.
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
1 participant
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.
Gallery::new called Path::is_file() on every directory entry, which is a stat syscall. DirEntry::file_type() answers the same question from the d_type field that readdir already returned, with no syscall.
Measured on two 50,000 file folders. 86.6 ms to 18.0 ms for 5,000 images among 45,000 other files. 104.3 ms to 22.3 ms for a folder of 50,000 images.
is_file() follows symlinks and file_type() does not, so a plain swap drops symlinked images from the gallery. The is_symlink() branch falls back to the stat for those entries only.
gallery.rs had no tests. Three were added, covering the filter rules, symlinked images, and dangling symlinks. Removing the symlink fallback fails two of them.