From 540a281ccda0f2dce928e630e5f43caa37271db3 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Tue, 13 Oct 2026 12:49:11 -0400 Subject: [PATCH] PERF-SYNC: a Synchronize Folder scan reads off the folder's listing whether a photo and its sidecar are there The scan asked the disk about every library photo in the folder: is its file there, and has it a sidecar under any of four names. On a network share each question is a round trip, and the scan had just listed the folder. Measured on a hard disk over SMB, for unchanged folders of 1,600 to 2,400 photos: 4.6 to 7.1 s of questions after a listing of 4 to 5 s. - The walk hands each folder's listing over (`walk::files_in_noting`: every entry's name and kind, hidden ones too, and whether that is all of them). The scan walks the folder once, then checks the photos beside the probing of new files. - `sync::Listing` answers "is it there" / "is it a file" where that holds on any file system: a name the folder lists is there; a plain ASCII name is not there when the folder was listed to its end, all its names are ASCII and none is that name in another case. Everything else is asked of the disk as before (a link, a name Windows reads another way, a folder that was not walked, and every name not listed in a folder that holds a name outside ASCII). Now under 0.05 s for those folders. Two things differ on purpose: a file the folder lists is there though it can't be opened by its path (a folder that may be listed but not searched, an over-long path), where such photos were listed as missing; and the answers are the folder as the walk saw it. Co-Authored-By: Claude Opus 5.5 --- crates/engine/examples/sync_scan_bench.rs | 47 ++++++ crates/engine/src/import.rs | 30 +++- crates/engine/src/sidecar.rs | 19 ++- crates/engine/src/sync.rs | 179 ++++++++++++++++++-- crates/engine/src/sync/tests_listing.rs | 191 ++++++++++++++++++++++ crates/engine/src/walk.rs | 97 ++++++++++- docs/whats-new.md | 17 ++ 7 files changed, 553 insertions(+), 27 deletions(-) create mode 100644 crates/engine/examples/sync_scan_bench.rs create mode 100644 crates/engine/src/sync/tests_listing.rs diff --git a/crates/engine/examples/sync_scan_bench.rs b/crates/engine/examples/sync_scan_bench.rs new file mode 100644 index 000000000..3b9bc51c1 --- /dev/null +++ b/crates/engine/examples/sync_scan_bench.rs @@ -0,0 +1,47 @@ +//! What a Synchronize Folder scan costs on a folder whose photos are all in the library and +//! unchanged: the walk, and the check of every photo and its sidecar. Reads the folder; writes +//! nothing there, and probes no file (the library is made from the folder's own listing). +//! +//! `cargo run --release -p lightcraft-engine --example sync_scan_bench -- ` +use std::time::Instant; + +use lightcraft_catalog::{Op, Photo, Source}; +use lightcraft_engine::Session; +use lightcraft_engine::sync::{SyncInput, SyncProgress, scan_with}; + +fn main() { + let Some(folder) = std::env::args().nth(1) else { + eprintln!("usage: sync_scan_bench "); + return; + }; + let mut s = Session::new(); + let t = Instant::now(); + let files = lightcraft_engine::import::expand(std::slice::from_ref(&folder), None); + println!("{} files listed in {:.1} s (the library is made of them)", files.len(), t.elapsed().as_secs_f64()); + for f in &files { + let id = s.catalog.alloc_photo_id(); + let name = std::path::Path::new(f).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_default(); + let p = Photo::new(id, Source::File { path: f.clone() }, &name, "JPEG", 6, 4, "2026-01-01T00:00:00"); + if s.catalog.apply(Op::AddPhoto { photo: Box::new(p) }).is_err() { + eprintln!("could not add {name}"); + } + } + for run in 0..2 { + let input = match SyncInput::new(&mut s, &folder, true) { + Ok(input) => input, + Err(e) => { + eprintln!("{e}"); + return; + } + }; + let t = Instant::now(); + let changes = scan_with(input, &SyncProgress::default()); + println!( + "run {run}: scan {:.1} s ({} new, {} missing, {} with sidecar news)", + t.elapsed().as_secs_f64(), + changes.new.len(), + changes.missing.len(), + changes.metadata.len() + ); + } +} diff --git a/crates/engine/src/import.rs b/crates/engine/src/import.rs index ecb80741e..821454c72 100644 --- a/crates/engine/src/import.rs +++ b/crates/engine/src/import.rs @@ -380,6 +380,17 @@ pub fn expand(paths: &[String], skip: Option<&Path>) -> Vec { /// [`expand`] with the walk bounded by `limits` → (files, whether a walk stopped at a bound). pub fn expand_within(paths: &[String], skip: Option<&Path>, limits: crate::walk::Limits) -> (Vec, bool) { + expand_noting(paths, skip, limits, None) +} + +/// [`expand_within`], handing the listing of every folder that is walked to `note` +/// ([`crate::walk::files_in_noting`]). +pub(crate) fn expand_noting( + paths: &[String], + skip: Option<&Path>, + limits: crate::walk::Limits, + mut note: Option, +) -> (Vec, bool) { let mut out = Vec::new(); let mut truncated = false; for p in paths { @@ -388,7 +399,10 @@ pub fn expand_within(paths: &[String], skip: Option<&Path>, limits: crate::walk: continue; } if p.is_dir() { - let w = crate::walk::files_in(p, skip, limits, is_supported); + let w = match note.as_mut() { + Some(note) => crate::walk::files_in_noting(p, skip, limits, is_supported, &mut **note), + None => crate::walk::files_in(p, skip, limits, is_supported), + }; if w.truncated { log::warn!( "import: stopped looking in {} after {} entries (limits: {} entries, {} folder levels)", @@ -630,6 +644,11 @@ impl ScanInput { (input, paths.to_vec()) } + /// The folder a scan never enters (the library's own). + pub(crate) fn skip(&self) -> Option<&Path> { + self.skip.as_deref() + } + /// The library as the scan sees it (shared, for [`crate::sync`], which looks at the same one). pub(crate) fn library(&self) -> std::sync::Arc { std::sync::Arc::clone(&self.library) @@ -648,9 +667,14 @@ pub fn scan(s: &mut Session, paths: &[String]) -> Vec { /// [`scan`] without the session, so it can run on a worker thread. Stops early (returning what it /// has) when `progress.cancel` is set. -pub fn scan_with(mut input: ScanInput, paths: &[String], progress: &ScanProgress) -> ScanOutput { - use std::sync::atomic::Ordering::Relaxed; +pub fn scan_with(input: ScanInput, paths: &[String], progress: &ScanProgress) -> ScanOutput { let files = expand(paths, input.skip.as_deref()); + scan_files(input, files, progress) +} + +/// [`scan_with`] for files that were found already (`paths` expanded, [`expand`]). +pub(crate) fn scan_files(mut input: ScanInput, files: Vec, progress: &ScanProgress) -> ScanOutput { + use std::sync::atomic::Ordering::Relaxed; let by_path = library_files(&input.library, &files); let todo: Vec = files.iter().filter(|f| !by_path.contains_key(Path::new(f))).cloned().collect(); progress.total.store(todo.len(), Relaxed); diff --git a/crates/engine/src/sidecar.rs b/crates/engine/src/sidecar.rs index c67a2c84f..41d1c5f57 100644 --- a/crates/engine/src/sidecar.rs +++ b/crates/engine/src/sidecar.rs @@ -65,15 +65,18 @@ pub fn sidecar_path(original: &str, naming: SidecarNaming) -> PathBuf { /// An existing sidecar for `original`: the preferred naming first, then the other one (also /// accepting an upper-case `.XMP`). pub fn find_sidecar(original: &str, naming: SidecarNaming) -> Option { + sidecar_candidates(original, naming).find(|p| p.is_file()) +} + +/// The names `original`'s sidecar can have, in the order they are looked for: under `naming`, +/// then under the other naming, each as `.xmp` and as `.XMP`. +pub(crate) fn sidecar_candidates(original: &str, naming: SidecarNaming) -> impl Iterator { let other = if naming == SidecarNaming::Stem { SidecarNaming::Full } else { SidecarNaming::Stem }; - [naming, other] - .into_iter() - .flat_map(|n| { - let p = sidecar_path(original, n); - let upper = p.with_extension("XMP"); - [p, upper] - }) - .find(|p| p.is_file()) + [naming, other].into_iter().flat_map(move |n| { + let p = sidecar_path(original, n); + let upper = p.with_extension("XMP"); + [p, upper] + }) } /// What a sidecar (or embedded XMP packet) says about a photo. `None` = not stated. diff --git a/crates/engine/src/sync.rs b/crates/engine/src/sync.rs index 4342ec6ad..6f236796e 100644 --- a/crates/engine/src/sync.rs +++ b/crates/engine/src/sync.rs @@ -38,7 +38,7 @@ //! not cancelled midway. use std::collections::HashMap; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::sync::Arc; use lightcraft_catalog::{Catalog, Filter, Op, Photo, PhotoId, Sort, Source}; @@ -203,18 +203,37 @@ pub fn scan_with(input: SyncInput, progress: &SyncProgress) -> FolderChanges { let namings = crate::sidecar::namings_in(&labels, naming, &ids); let photos: Vec<(Arc, SidecarNaming)> = ids.iter().zip(namings).filter_map(|(id, naming)| labels.photo(*id).map(|p| (Arc::clone(p), naming))).collect(); + // The folder is walked once, and its listing answers for the library's photos in it: on a + // network share that is one request for a folder's names where it was several for each + // photo. Then both halves wait on the disk at once: the new files are probed (on their own + // threads) beside the checks of the photos, which ask the disk only what the listing can't say. + // (how many photos there are to check is known before the walk: the count shows while it runs) + progress.to_check.store(photos.len(), std::sync::atomic::Ordering::Relaxed); + let mut listing = Listing::default(); + let (files, _) = crate::import::expand_noting( + std::slice::from_ref(&folder), + scan.skip(), + crate::walk::Limits::default(), + Some(&mut |dir, names, complete| listing.note(dir, names, complete)), + ); + let listing = &listing; // (no threads on the web: one after the other there) #[cfg(target_arch = "wasm32")] - let (out, checks) = (crate::import::scan_with(scan, std::slice::from_ref(&folder), &progress.files), check_photos(&photos, &labels, progress)); + let (out, checks) = (crate::import::scan_files(scan, files, &progress.files), check_photos(&photos, &labels, listing, progress)); #[cfg(not(target_arch = "wasm32"))] let (out, checks) = std::thread::scope(|sc| { let photos = &photos; let labels = &labels; - let checks = sc.spawn(move || check_photos(photos, labels, progress)); - let out = crate::import::scan_with(scan, std::slice::from_ref(&folder), &progress.files); + let checks = sc.spawn(move || check_photos(photos, labels, listing, progress)); + let out = crate::import::scan_files(scan, files, &progress.files); // (a check that panicked counts as nothing found; the scan still reports the rest) (out, checks.join().unwrap_or_default()) }); + log::debug!( + "synchronize: {} folders listed; the disk was asked {} times about the library's photos", + listing.folders.len(), + listing.asked.load(std::sync::atomic::Ordering::Relaxed) + ); let mut changes = FolderChanges { path: folder, seen, ..Default::default() }; for ((p, _), check) in photos.iter().zip(checks) { let Some(file) = crate::cmd::missing::checked_path(p) else { continue }; @@ -335,11 +354,136 @@ enum Check { News, } -fn check_photo(p: &Photo, naming: SidecarNaming, labels: &Catalog) -> Check { +/// The listings of the folders a scan walked: what is in each, by name. It answers "is this file +/// there" for the library's photos and their sidecars without asking the disk, which on a +/// network share is a round trip for every name (and a photo without a sidecar took five). +/// +/// It answers only what holds on any file system. A name is there when the folder lists exactly +/// that name ([`asks_the_disk`] aside). It is not there when the folder was listed to its end, +/// every name in the folder is ASCII, the name asked about is plain ASCII ([`plain_name`]), and +/// no name in the folder is that name in another case. Anything else is not answered, and the +/// disk is asked as before: a name that differs in case only, a link, a folder that was not +/// walked or not to its end, and every name that is not listed in a folder that holds a name +/// outside ASCII. (File systems hold names outside ASCII equal in more ways than can be listed +/// here: by case, where the capital sharp s is `SS`; by how accents are written; and some pass +/// over marks altogether.) +/// +/// Where the listing and a look-up would differ, the listing is the folder as the scan saw it: +/// a file it lists is there though it could not be opened by its path (a folder that may be +/// listed but not searched, a path longer than the system takes), and a share that drops after +/// the walk does not turn every photo the listing answers for into a missing one. +#[derive(Default)] +pub(crate) struct Listing { + folders: HashMap, + /// How often the disk was asked after all (for the tests, and the log). + pub(crate) asked: std::sync::atomic::AtomicUsize, +} + +/// One folder's listing. +struct Listed { + /// name → what it is. + names: HashMap, + /// The names, lower-cased (only kept for a folder whose names are all ASCII): what a plain + /// name asked about could be in another case. + lower: std::collections::HashSet, + /// Every entry was read, and every name is ASCII: a plain name that is not listed, in any + /// case, is not there. + sure: bool, +} + +/// Whether the disk is asked about `name` whatever the listing holds: a name Windows takes for +/// a device (`NUL`, `aux.jpg`: there in every folder, or not openable where it is listed). +fn asks_the_disk(name: &str) -> bool { + let stem = name.split('.').next().unwrap_or_default().trim_end_matches(' ').to_ascii_lowercase(); + // (`COM` and `LPT` with any one digit, the superscript ones Windows also takes among them) + let numbered = ["com", "lpt"].iter().any(|port| { + let mut rest = stem.strip_prefix(port).unwrap_or_default().chars(); + matches!((rest.next(), rest.next()), (Some(d), None) if d.is_ascii_digit() || ['\u{b9}', '\u{b2}', '\u{b3}'].contains(&d)) + }); + numbered || ["con", "prn", "aux", "nul", "conin$", "conout$", "clock$"].contains(&stem.as_str()) +} + +/// Whether `name` can only be found under itself in some case: ASCII, and nothing in it that a +/// file system resolves another way (a short name with `~`, a stream after `:`, dots or spaces +/// at its end, which Windows drops). +fn plain_name(name: &str) -> bool { + name.is_ascii() && !name.contains(['~', ':']) && !name.ends_with(['.', ' ']) && !name.is_empty() +} + +impl Listing { + /// Take the listing of `folder` from the walk ([`crate::walk::files_in_noting`]). + pub(crate) fn note(&mut self, folder: &Path, entries: Vec<(std::ffi::OsString, crate::walk::EntryKind)>, complete: bool) { + let mut sure = complete; + let mut names = HashMap::with_capacity(entries.len()); + let mut lower = std::collections::HashSet::new(); + for (name, kind) in entries { + match name.into_string() { + Ok(name) => { + sure &= name.is_ascii(); + lower.insert(name.to_ascii_lowercase()); + names.insert(name, kind); + } + // (a name that is no text is not one a photo's path can be compared with) + Err(_) => sure = false, + } + } + if !sure { + lower.clear(); + } + self.folders.insert(folder.to_path_buf(), Listed { names, lower, sure }); + } + + /// What the listing of `path`'s folder says of it: its kind when it is listed under exactly + /// this name, `Some(None)` when it is surely not there, `None` when the listing can't say. + fn kind(&self, path: &Path) -> Option> { + let name = path.file_name()?.to_str()?; + // (the path ends in that name: no `/.` or separator after it, which the disk would resolve) + if !path.to_str()?.ends_with(name) { + return None; + } + if asks_the_disk(name) { + return None; + } + let folder = self.folders.get(path.parent()?)?; + if let Some(kind) = folder.names.get(name) { + return Some(Some(*kind)); + } + (folder.sure && plain_name(name) && !folder.lower.contains(&name.to_ascii_lowercase())).then_some(None) + } + + /// `Path::exists`, where the listing can say. + pub(crate) fn exists(&self, path: &Path) -> Option { + match self.kind(path)? { + Some(crate::walk::EntryKind::Other) => None, + Some(_) => Some(true), + None => Some(false), + } + } + + /// `Path::is_file`, where the listing can say. + pub(crate) fn is_file(&self, path: &Path) -> Option { + match self.kind(path)? { + Some(crate::walk::EntryKind::Other) => None, + Some(kind) => Some(kind == crate::walk::EntryKind::File), + None => Some(false), + } + } + + /// The listing's answer, or the disk's where it has none. + fn or_ask(&self, answer: Option, ask: impl FnOnce() -> bool) -> bool { + answer.unwrap_or_else(|| { + self.asked.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + ask() + }) + } +} + +fn check_photo(p: &Photo, naming: SidecarNaming, labels: &Catalog, listing: &Listing) -> Check { let Some(file) = crate::cmd::missing::checked_path(p) else { return Check::Nothing }; - if !Path::new(file).exists() { + let file = Path::new(file); + if !listing.or_ask(listing.exists(file), || file.exists()) { Check::Missing - } else if p.copy_of.is_none() && sidecar_has_news(labels, p, naming) { + } else if p.copy_of.is_none() && sidecar_has_news(labels, p, naming, listing) { Check::News } else { Check::Nothing @@ -348,12 +492,12 @@ fn check_photo(p: &Photo, naming: SidecarNaming, labels: &Catalog) -> Check { /// [`check_photo`] for each photo, in order, on several threads (each file is a wait on the /// disk, not work for the CPU). Photos not reached before a cancel count as nothing found. -fn check_photos(photos: &[(Arc, SidecarNaming)], labels: &Catalog, progress: &SyncProgress) -> Vec { +fn check_photos(photos: &[(Arc, SidecarNaming)], labels: &Catalog, listing: &Listing, progress: &SyncProgress) -> Vec { use std::sync::atomic::Ordering::Relaxed; progress.to_check.store(photos.len(), Relaxed); let cancel = &progress.files.cancel; let check = |p: &Photo, naming: SidecarNaming| { - let c = check_photo(p, naming, labels); + let c = check_photo(p, naming, labels, listing); progress.checked.fetch_add(1, Relaxed); c }; @@ -421,10 +565,17 @@ impl Session { /// Whether `p`'s XMP sidecar file was saved after the library last had its say about the photo /// (imported, or edited here) and says something the library doesn't. -fn sidecar_has_news(labels: &Catalog, p: &Photo, naming: SidecarNaming) -> bool { +fn sidecar_has_news(labels: &Catalog, p: &Photo, naming: SidecarNaming, listing: &Listing) -> bool { let Source::File { path } = &p.source else { return false }; - let Some(file) = crate::sidecar::find_sidecar(path, naming) else { return false }; - let Ok(meta) = std::fs::metadata(&file) else { return false }; + // ([`crate::sidecar::find_sidecar`], each name looked up in the listing before the disk) + // (a name the listing holds is read for its date next, and one that can't be read is passed + // over for the next name, as a name that is no file was) + let found = crate::sidecar::sidecar_candidates(path, naming).find_map(|c| { + let there = listing.or_ask(listing.is_file(&c), || c.is_file()); + let meta = there.then(|| std::fs::metadata(&c).ok()).flatten().filter(std::fs::Metadata::is_file)?; + Some((c, meta)) + }); + let Some((file, meta)) = found else { return false }; if meta.len() > MAX_SIDECAR_BYTES { return false; } @@ -708,3 +859,7 @@ impl SyncCommit { self.report } } + +#[cfg(test)] +#[path = "sync/tests_listing.rs"] +mod tests_listing; diff --git a/crates/engine/src/sync/tests_listing.rs b/crates/engine/src/sync/tests_listing.rs new file mode 100644 index 000000000..e54ed76ef --- /dev/null +++ b/crates/engine/src/sync/tests_listing.rs @@ -0,0 +1,191 @@ +//! Synchronize Folder answers "is the photo's file there" and "has it a sidecar" from the +//! listing of the folder its scan has just made, instead of asking the disk about every photo. +//! +//! On a network share each question is a round trip (about 5 ms a file on a hard disk over SMB, +//! measured), and a photo without a sidecar took five of them; the listing has every name. +//! The listing answers only what it can be sure of on any file system: where a name could match +//! another way (case, letters outside ASCII, a link), the disk is asked as before. +use super::*; +use crate::walk::EntryKind::{File, Folder, Other}; + +fn listing(folders: &[(&str, &[(&str, crate::walk::EntryKind)], bool)]) -> Listing { + let mut l = Listing::default(); + for (dir, names, complete) in folders { + l.note(Path::new(dir), names.iter().map(|(n, k)| (std::ffi::OsString::from(n), *k)).collect(), *complete); + } + l +} + +/// Given the listing of a folder, when it is asked about a name, then it says "there" for a +/// name it lists, "not there" for one nothing in the folder could be taken for, and nothing +/// (ask the disk) wherever a file system could see it otherwise. +#[test] +fn the_listing_answers_only_what_it_is_sure_of() { + let l = listing(&[ + ( + "/pics/trip", + &[("IMG_1.CR3", File), ("IMG_1.xmp", File), ("img_2.jpg", File), ("sub", Folder), ("link.jpg", Other), (".hidden.xmp", File)], + true, + ), + ("/pics/cut", &[("IMG_1.CR3", File)], false), + ("/pics/names", &[("IMG_1.CR3", File), ("Größe.JPG", File)], true), + // (the capital sharp s, which a file system that folds case takes for `SS`) + ("/pics/sharp", &[("IMG_1.CR3", File), ("STRA\u{1e9e}E.jpg", File)], true), + ("/pics/devices", &[("aux.jpg", File), ("COM\u{b9}.jpg", File), ("IMG_1.CR3", File)], true), + ]); + let (there, gone, ask) = (Some(true), Some(false), None); + for (path, exists, is_file) in [ + ("/pics/trip/IMG_1.CR3", there, there), + ("/pics/trip/IMG_1.xmp", there, there), + ("/pics/trip/.hidden.xmp", there, there), + ("/pics/trip/sub", there, gone), + ("/pics/trip/IMG_9.CR3", gone, gone), + ("/pics/trip/IMG_1.XMP", ask, ask), + ("/pics/trip/IMG_2.JPG", ask, ask), + ("/pics/trip/link.jpg", ask, ask), + ("/pics/trip/SUB", ask, ask), + // a folder the walk did not list to its end, or not at all + ("/pics/cut/IMG_1.CR3", there, there), + ("/pics/cut/IMG_2.CR3", ask, ask), + ("/pics/other/IMG_1.CR3", ask, ask), + // a folder that holds a name outside ASCII: what it lists is there, and nothing is surely not there (file + // systems hold such names equal in more ways than one) + ("/pics/names/IMG_1.CR3", there, there), + ("/pics/names/Größe.JPG", there, there), + ("/pics/names/IMG_2.CR3", ask, ask), + ("/pics/names/größe.JPG", ask, ask), + ("/pics/trip/Größe.JPG", ask, ask), + ("/pics/sharp/strasse.jpg", ask, ask), + ("/pics/sharp/STRASSE.xmp", ask, ask), + ("/pics/sharp/IMG_2.CR3", ask, ask), + // names Windows resolves another way: a short name, a stream, dots or spaces at the end, a device + ("/pics/trip/IMG_10~1.CR3", ask, ask), + ("/pics/trip/IMG_1.CR3::$DATA", ask, ask), + ("/pics/trip/IMG_1.CR3.", ask, ask), + ("/pics/trip/IMG_1.CR3 ", ask, ask), + ("/pics/devices/aux.jpg", ask, ask), + ("/pics/devices/NUL", ask, ask), + ("/pics/devices/nul.xmp", ask, ask), + ("/pics/devices/COM1 .jpg", ask, ask), + ("/pics/devices/COM\u{b9}.jpg", ask, ask), + ("/pics/trip/con", ask, ask), + ("/pics/trip/PRN.txt", ask, ask), + ("/pics/trip/com2.jpg", ask, ask), + ("/pics/trip/LPT9.xmp", ask, ask), + ("/pics/trip/lpt0", ask, ask), + ("/pics/trip/nul.tar.gz", ask, ask), + ("/pics/trip/CONIN$", ask, ask), + ("/pics/trip/com10.jpg", gone, gone), + ("/pics/trip/computer.jpg", gone, gone), + ("/pics/devices/IMG_1.CR3", there, there), + ("/pics/devices/auxiliary.jpg", ask, ask), + ("/pics/trip/auxiliary.jpg", gone, gone), + // a path that does not end in its file's name is the disk's to resolve + ("/pics/trip/IMG_1.CR3/", ask, ask), + ("/pics/trip/IMG_1.CR3/.", ask, ask), + ("/pics/trip/", ask, ask), + ("", ask, ask), + ] { + assert_eq!((l.exists(Path::new(path)), l.is_file(Path::new(path))), (exists, is_file), "{path:?}"); + } + // a name that is no text: nothing in its folder is surely not there + #[cfg(unix)] + { + use std::os::unix::ffi::OsStringExt; + let mut l = Listing::default(); + l.note( + Path::new("/pics/raw"), + vec![(std::ffi::OsString::from("IMG_1.CR3"), File), (std::ffi::OsString::from_vec(vec![0x66, 0xff, 0x6f]), File)], + true, + ); + assert_eq!((l.exists(Path::new("/pics/raw/IMG_1.CR3")), l.exists(Path::new("/pics/raw/IMG_2.CR3"))), (there, ask)); + } +} + +fn photo(c: &mut Catalog, path: &Path) -> Arc { + let id = c.alloc_photo_id(); + let name = path.file_name().unwrap().to_string_lossy().to_string(); + c.apply(lightcraft_catalog::Op::AddPhoto { + photo: Box::new(Photo::new(id, Source::File { path: path.to_string_lossy().to_string() }, &name, "JPEG", 6, 4, "2020-01-01T00:00:00")), + }) + .unwrap(); + Arc::clone(c.photo(id).unwrap()) +} + +/// Given a folder on disk whose photos are there or gone, with sidecars under every name a +/// sidecar can have, when each photo is checked with the folder's listing, then the answer is +/// the one the disk gives; and for photos with plain names the disk was asked nothing but the +/// sidecars' dates. +#[test] +fn a_check_from_the_listing_is_the_check_from_the_disk() { + let dir = std::env::temp_dir().join(format!("lc-listing-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(dir.join("sub")).unwrap(); + let news = r#""#; + for name in [ + "plain.jpg", + "stem.jpg", + "stem.xmp", + "full.jpg", + "full.jpg.xmp", + "upper.jpg", + "upper.XMP", + "sub/deep.jpg", + "sub/deep.xmp", + "Größe.jpg", + "Größe.xmp", + "dir.jpg.xmp", + ] { + std::fs::write(dir.join(name), if name.ends_with("mp") || name.ends_with("MP") { news.as_bytes() } else { b"jpeg" }).unwrap(); + } + let mut c = Catalog::new(); + let photos: Vec> = + ["plain.jpg", "stem.jpg", "full.jpg", "upper.jpg", "sub/deep.jpg", "Größe.jpg", "gone.jpg", "sub/gone.jpg", "dir.jpg"] + .iter() + .map(|n| photo(&mut c, &dir.join(n))) + .collect(); + // (`dir.jpg` is gone, though a sidecar of its name is there) + let mut l = Listing::default(); + let walked = + crate::walk::files_in_noting(&dir, None, crate::walk::Limits::default(), |_| true, &mut |d, names, complete| l.note(d, names, complete)); + assert_eq!(walked.files.len(), 12); + let nothing = Listing::default(); + for naming in [SidecarNaming::Stem, SidecarNaming::Full] { + for p in &photos { + let from_disk = check_photo(p, naming, &c, ¬hing); + assert_eq!(check_photo(p, naming, &c, &l), from_disk, "{:?} {naming:?}", p.file_name); + } + } + let expected = [Check::Nothing, Check::News, Check::News, Check::News, Check::News, Check::News, Check::Missing, Check::Missing, Check::Missing]; + assert_eq!(photos.iter().map(|p| check_photo(p, SidecarNaming::Stem, &c, &l)).collect::>(), expected); + + // what the disk was asked about the same photos, with the listing and without: fewer questions with it + let asked = |l: &Listing| { + for p in &photos { + check_photo(p, SidecarNaming::Stem, &c, l); + } + l.asked.load(std::sync::atomic::Ordering::Relaxed) + }; + let mut with = Listing::default(); + crate::walk::files_in_noting(&dir, None, crate::walk::Limits::default(), |_| true, &mut |d, names, complete| with.note(d, names, complete)); + let (with, without) = (asked(&with), asked(&Listing::default())); + // (without: each photo's file, and the names of its sidecar up to the one that is there: 20 questions where + // names are found in any case, 21 where they are not) + assert!(without >= 20, "{without}"); + // (with: this folder holds `Größe.*`, so what it does not list is still asked about: the four names of the + // sidecar `plain.jpg` has not, the two `full.jpg` has not, `upper.xmp`, and the two photos that are gone. What + // it lists is not asked about, nor anything in `sub`, whose names are all ASCII) + assert_eq!(with, 9, "asked {with} times with the listing, {without} without"); + + // a folder of plain names only: the disk is asked nothing at all + let sub = dir.join("sub"); + let mut only_plain = Listing::default(); + crate::walk::files_in_noting(&sub, None, crate::walk::Limits::default(), |_| true, &mut |d, names, complete| only_plain.note(d, names, complete)); + let mut c2 = Catalog::new(); + for (name, expected) in [("deep.jpg", Check::News), ("gone.jpg", Check::Missing), ("gone.jpg.xmp", Check::Missing)] { + let p = photo(&mut c2, &sub.join(name)); + assert_eq!(check_photo(&p, SidecarNaming::Stem, &c2, &only_plain), expected, "{name}"); + } + assert_eq!(only_plain.asked.load(std::sync::atomic::Ordering::Relaxed), 0, "every answer came from the listing"); + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/crates/engine/src/walk.rs b/crates/engine/src/walk.rs index 1c1b53b4a..f8a3ba251 100644 --- a/crates/engine/src/walk.rs +++ b/crates/engine/src/walk.rs @@ -47,10 +47,35 @@ fn hidden(p: &Path) -> bool { p.file_name().is_some_and(|n| n.to_string_lossy().starts_with('.')) } +/// What a folder's listing says an entry is. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum EntryKind { + File, + Folder, + /// A link, or an entry whose kind the listing did not say. + Other, +} + +/// What [`files_in_noting`] hands over for each folder it lists: the folder, the name and kind +/// of every entry in it (hidden ones too), and whether that is all of them (not when the walk +/// stopped inside the folder, or an entry could not be read). +pub type NoteFolder<'a> = &'a mut dyn FnMut(&Path, Vec<(std::ffi::OsString, EntryKind)>, bool); + /// The files below folder `root` that `keep` accepts. Hidden entries (names starting with `.`) /// are left out, the folder `skip` (e.g. the library's own) is never entered, and the walk stops /// at `limits` (see [`Walked::truncated`]). Unreadable folders are skipped. -pub fn files_in(root: &Path, skip: Option<&Path>, limits: Limits, mut keep: impl FnMut(&Path) -> bool) -> Walked { +pub fn files_in(root: &Path, skip: Option<&Path>, limits: Limits, keep: impl FnMut(&Path) -> bool) -> Walked { + walk(root, skip, limits, keep, None) +} + +/// [`files_in`], handing the listing of every folder it reads to `note`: who walks a folder to +/// find its files has, with that, the answer to "is this name there" for everything in it, and +/// need not ask the disk again (a round trip for each name on a network share). +pub fn files_in_noting(root: &Path, skip: Option<&Path>, limits: Limits, keep: impl FnMut(&Path) -> bool, note: NoteFolder) -> Walked { + walk(root, skip, limits, keep, Some(note)) +} + +fn walk(root: &Path, skip: Option<&Path>, limits: Limits, mut keep: impl FnMut(&Path) -> bool, mut note: Option) -> Walked { let mut out = Walked::default(); // canonical folders reached through a link, so a link loop is walked at most once let mut linked: HashSet = HashSet::new(); @@ -77,17 +102,41 @@ pub fn files_in(root: &Path, skip: Option<&Path>, limits: Limits, mut keep: impl } let Ok(rd) = std::fs::read_dir(&path) else { continue }; let mut children: Vec<(PathBuf, bool)> = Vec::new(); - for e in rd.flatten() { + // (the folder's listing, for who asked for it; all of it unless something below says otherwise) + let mut listed = note.is_some().then(Vec::new); + let mut complete = true; + for e in rd { + let Ok(e) = e else { + complete = false; + continue; + }; if out.entries >= limits.max_entries { out.truncated = true; + complete = false; break; } out.entries = out.entries.saturating_add(1); let p = e.path(); - if hidden(&p) { + // (a hidden entry's kind is only asked for when the listing is wanted; every other + // entry's is asked once) + let is_hidden = hidden(&p); + if is_hidden && listed.is_none() { continue; } - let Ok(ft) = e.file_type() else { continue }; + let ft = e.file_type(); + if let Some(listed) = listed.as_mut() { + let kind = match &ft { + Ok(t) if t.is_symlink() => EntryKind::Other, + Ok(t) if t.is_dir() => EntryKind::Folder, + Ok(t) if t.is_file() => EntryKind::File, + _ => EntryKind::Other, + }; + listed.push((e.file_name(), kind)); + } + if is_hidden { + continue; + } + let Ok(ft) = ft else { continue }; let dir = if ft.is_symlink() { // a link (or a Windows junction): a folder is entered once, a file is a file match std::fs::metadata(&p) { @@ -105,6 +154,9 @@ pub fn files_in(root: &Path, skip: Option<&Path>, limits: Limits, mut keep: impl }; children.push((p, dir)); } + if let (Some(note), Some(listed)) = (note.as_mut(), listed) { + note(&path, listed, complete); + } children.sort(); let below = depth.saturating_add(1); stack.extend(children.into_iter().rev().map(|(p, d)| (p, below, d))); @@ -123,6 +175,43 @@ mod tests { d } + /// Given a folder with files, a folder inside it, a hidden file and a link, when it is walked + /// with its listings noted, then the walk finds what it finds without them, and each folder's + /// listing holds every entry by name with its kind (hidden ones too, a link as neither file nor + /// folder) and says it is all of them; a walk that stops inside a folder says it is not. + #[test] + fn a_walk_notes_each_folders_listing() { + let root = tmp("noting"); + std::fs::create_dir_all(root.join("sub")).unwrap(); + for name in ["a.jpg", ".hidden", "sub/b.jpg"] { + std::fs::write(root.join(name), b"x").unwrap(); + } + #[cfg(unix)] + std::os::unix::fs::symlink(root.join("a.jpg"), root.join("link.jpg")).unwrap(); + let mut noted: Vec<(PathBuf, Vec<(String, EntryKind)>, bool)> = Vec::new(); + let walked = files_in_noting(&root, None, Limits::default(), |_| true, &mut |dir, entries, complete| { + let mut entries: Vec<(String, EntryKind)> = entries.into_iter().map(|(n, k)| (n.to_string_lossy().to_string(), k)).collect(); + entries.sort_by(|a, b| a.0.cmp(&b.0)); + noted.push((dir.to_path_buf(), entries, complete)); + }); + assert_eq!(walked, files_in(&root, None, Limits::default(), |_| true), "the same walk, noted or not"); + let named = |n: &str, k: EntryKind| (n.to_string(), k); + let mut top = vec![named(".hidden", EntryKind::File), named("a.jpg", EntryKind::File), named("sub", EntryKind::Folder)]; + if cfg!(unix) { + top.insert(2, named("link.jpg", EntryKind::Other)); + } + assert_eq!(noted, [(root.clone(), top, true), (root.join("sub"), vec![named("b.jpg", EntryKind::File)], true)]); + + // stopped inside the first folder: its listing is not all of it + let mut cut = Vec::new(); + let walked = files_in_noting(&root, None, Limits { max_depth: 8, max_entries: 2 }, |_| true, &mut |dir, entries, complete| { + cut.push((dir.to_path_buf(), entries.len(), complete)) + }); + assert!(walked.truncated); + assert_eq!(cut.first().map(|(dir, n, complete)| (dir.clone(), *n, *complete)), Some((root.clone(), 2, false))); + let _ = std::fs::remove_dir_all(&root); + } + #[test] fn files_come_in_path_order_without_hidden_ones_or_the_skipped_folder() { let d = tmp("order"); diff --git a/docs/whats-new.md b/docs/whats-new.md index 1bf44c3f4..591513d84 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -53,6 +53,23 @@ else (issue #1504): trimming or clearing the cache can't delete files outside it, and thumbnails are never read from or written through such a link. +### Synchronize Folder asks a network share less +- A Synchronize Folder scan no longer asks the disk about every photo of the library in the folder (is its file + there, has it a sidecar): it has just listed the folder, and reads the answers off the listing. On a network share + each of those questions is a round trip. Measured on a hard disk over SMB, for folders of 1,600 to 2,400 photos + that had not changed: 4.6 to 7.1 s of questions after the listing (about 3 ms a photo), now under 0.05 s. The + listing itself took 4 to 5 s and is as it was. +- The disk is still asked wherever a listing can't be sure what a file system would say: about a name that is + there in another case, a link, a name Windows reads another way (`aux.jpg`, a `~`, a `:`, a dot or a space at the + end), a folder the scan did not walk to its end, and, in a folder that holds any name with a character outside + ASCII (an umlaut, a curly apostrophe, a dash), about every name the folder does not list. Names it does list are + answered there too. In such a folder the scan saves less, and its checks now start after the listing where they + ran beside it. +- A photo the folder lists is there for the scan, also where its file could not be opened by its path (a folder + that may be listed but not searched, a path longer than the system takes): such photos used to be listed as + missing. And a share that drops after the scan has listed a folder no longer turns that folder's photos into + missing ones. + ### Large libraries: Synchronize Folder, import and sidecars start sooner - Opening Synchronize Folder, an import review or an import no longer makes a table of every photo in the library before it looks at the first file. In a library of 3.5 million photos that held the window for about 6 s before