From c208859ec208943594b1988695ba72ffe4b80832 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Fri, 30 Oct 2026 07:36:21 -0400 Subject: [PATCH] PERF-SMART: a filePath rule over a non-ASCII path folds case in stack buffers instead of allocating two strings per photo --- Cargo.lock | 1 + crates/catalog/Cargo.toml | 1 + crates/catalog/src/rules.rs | 76 ++++++++++- crates/catalog/src/rules/tests_path_fold.rs | 135 ++++++++++++++++++++ crates/catalog/tests/path_fold_heap.rs | 59 +++++++++ 5 files changed, 268 insertions(+), 4 deletions(-) create mode 100644 crates/catalog/src/rules/tests_path_fold.rs create mode 100644 crates/catalog/tests/path_fold_heap.rs diff --git a/Cargo.lock b/Cargo.lock index 17f738ea5..22f19ffce 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3567,6 +3567,7 @@ version = "0.15.0" dependencies = [ "chrono", "crc32fast", + "dhat", "lightcraft-develop", "lightcraft-geom", "lightcraft-meta", diff --git a/crates/catalog/Cargo.toml b/crates/catalog/Cargo.toml index b12e18803..aecd1f97d 100644 --- a/crates/catalog/Cargo.toml +++ b/crates/catalog/Cargo.toml @@ -24,6 +24,7 @@ web-time = { workspace = true } winapi-util = { workspace = true } [dev-dependencies] +dhat = { workspace = true } lightcraft-geom = { workspace = true } lightcraft-testkit = { workspace = true } proptest = { workspace = true } diff --git a/crates/catalog/src/rules.rs b/crates/catalog/src/rules.rs index ec92dc724..8392a4c7a 100644 --- a/crates/catalog/src/rules.rs +++ b/crates/catalog/src/rules.rs @@ -901,9 +901,11 @@ fn path_bytes_contains(hay: &[u8], needle: &[u8]) -> bool { tail.iter().enumerate().any(|(j, &c)| at(tail_from + j, c)) } -/// A `filePath` rule. The common case (an ASCII path and an ASCII text) is compared in place, folding -/// case and `\` as it goes, with nothing allocated per photo; any other text takes -/// [`file_path_matches_slow`], so Unicode lower-casing (ß, İ, final sigma…) behaves as it always did. +/// A `filePath` rule. An ASCII path and an ASCII text are compared as bytes, a path or text with other +/// characters folded into stack buffers ([`file_path_matches_folded`]); both fold case and `\` as they +/// go, with nothing allocated per photo. Only what that can't reproduce (a text that is not a string, a +/// path or text longer than the buffers, or a capital sigma, whose lower case depends on its neighbors) +/// takes [`file_path_matches_slow`], so Unicode lower-casing (ß, İ, final sigma…) behaves as it always did. fn file_path_matches(op: &str, source: &Source, value: &Value) -> bool { let path = match source { Source::File { path } => path.as_str(), @@ -911,7 +913,7 @@ fn file_path_matches(op: &str, source: &Source, value: &Value) -> bool { }; let Value::String(want) = value else { return file_path_matches_slow(op, path, value) }; if !path.is_ascii() || !want.is_ascii() { - return file_path_matches_slow(op, path, value); + return file_path_matches_folded(op, path, want).unwrap_or_else(|| file_path_matches_slow(op, path, value)); } let (path, want) = (path.trim().as_bytes(), want.trim().as_bytes()); match op { @@ -928,6 +930,70 @@ fn file_path_matches(op: &str, source: &Source, value: &Value) -> bool { } } +/// Room for a folded path (a path of 1 KiB; the fold of a character is at most 1.5 times its bytes) and a folded text. +const FOLDED_PATH_BYTES: usize = 1536; +const FOLDED_TEXT_BYTES: usize = 384; +/// The pair tried first. +const SMALL_PATH_BYTES: usize = 256; +const SMALL_TEXT_BYTES: usize = 96; + +/// `s` as the `filePath` rules compare it, written into `buf`: `\` as `/`, every character lower-cased +/// the way [`str::to_lowercase`] does (`İ` becomes `i` and U+0307). `None` when it doesn't fit, or for a +/// capital sigma, which `to_lowercase` writes as `ς` at the end of a word and `σ` elsewhere, by Unicode +/// tables this module can't reach; the caller takes the allocating comparison. +fn fold_path<'b>(s: &str, buf: &'b mut [u8]) -> Option<&'b str> { + let (mut n, mut at) = (0, 0); + while at < s.len() { + // an ASCII run is copied and folded as one block + let run = s.as_bytes()[at..].iter().position(|b| !b.is_ascii()).unwrap_or(s.len() - at); + if run > 0 { + let out = buf.get_mut(n..n + run)?; + out.copy_from_slice(&s.as_bytes()[at..at + run]); + out.iter_mut().for_each(|b| *b = fold_path_byte(*b)); + (n, at) = (n + run, at + run); + continue; + } + let c = s[at..].chars().next()?; + if c == '\u{3a3}' { + return None; + } + for l in c.to_lowercase() { + n += l.encode_utf8(buf.get_mut(n..)?.get_mut(..l.len_utf8())?).len(); + } + at += c.len_utf8(); + } + std::str::from_utf8(&buf[..n]).ok() +} + +/// [`file_path_matches`] for a path or text with non-ASCII characters: both folded into stack buffers, then +/// compared as the allocating comparison does, so the answers are the same by construction (a match may +/// begin inside one character's several lower-case characters, `İ` then U+0307, as it does there). +/// Photo paths are short, so a small pair of buffers goes first (clearing a big one costs more than a +/// short path's fold); `None` where [`fold_path`] can't fold them in the large pair either. +fn file_path_matches_folded(op: &str, path: &str, want: &str) -> Option { + file_path_matches_folded_in::<{ SMALL_PATH_BYTES }, { SMALL_TEXT_BYTES }>(op, path, want) + .or_else(|| file_path_matches_folded_in::<{ FOLDED_PATH_BYTES }, { FOLDED_TEXT_BYTES }>(op, path, want)) +} + +fn file_path_matches_folded_in(op: &str, path: &str, want: &str) -> Option { + if matches!(op, "isEmpty" | "isNotEmpty") { + return Some((op == "isEmpty") == path.trim().is_empty()); + } + let mut want_buf = [0u8; TEXT]; + let mut path_buf = [0u8; PATH]; + let want = fold_path(want.trim(), &mut want_buf)?; + Some(match op { + // the whole string, spaces included (not word by word like the other text fields) + "contains" => !want.is_empty() && fold_path(path, &mut path_buf)?.contains(want), + "notContains" => want.is_empty() || !fold_path(path, &mut path_buf)?.contains(want), + "is" => fold_path(path.trim(), &mut path_buf)? == want, + "isNot" => fold_path(path.trim(), &mut path_buf)? != want, + "startsWith" => fold_path(path.trim(), &mut path_buf)?.starts_with(want), + "endsWith" => fold_path(path.trim(), &mut path_buf)?.ends_with(want), + _ => false, + }) +} + /// [`file_path_matches`] for any text: the path and the text are normalised into new strings. fn file_path_matches_slow(op: &str, path: &str, value: &Value) -> bool { // `/` and `\` both separate, whatever platform the catalog came from; demo photos have no path @@ -1180,3 +1246,5 @@ impl RuleSet { #[cfg(test)] mod tests; +#[cfg(test)] +mod tests_path_fold; diff --git a/crates/catalog/src/rules/tests_path_fold.rs b/crates/catalog/src/rules/tests_path_fold.rs new file mode 100644 index 000000000..fbb5d45f2 --- /dev/null +++ b/crates/catalog/src/rules/tests_path_fold.rs @@ -0,0 +1,135 @@ +//! A `filePath` rule over a non-ASCII path or text compares in place, with the answers the +//! allocating comparison ([`file_path_matches_slow`]) always gave. That nothing is allocated per +//! photo is checked in `tests/path_fold_heap.rs` (dhat's profiler is global, so it has its own binary). + +use proptest::prelude::*; +use serde_json::{Value, json}; + +use super::*; +use crate::Source; + +const OPS: [&str; 9] = ["contains", "notContains", "is", "isNot", "startsWith", "endsWith", "isEmpty", "isNotEmpty", "bogus"]; + +fn file(path: &str) -> Source { + Source::File { path: path.to_string() } +} + +fn fast(op: &str, path: &str, want: &str) -> bool { + file_path_matches(op, &file(path), &Value::String(want.to_string())) +} + +fn old(op: &str, path: &str, want: &str) -> bool { + file_path_matches_slow(op, path, &Value::String(want.to_string())) +} + +#[test] +fn pinned_unicode_cases_keep_their_old_answers() { + // (path, text) pairs where lower-casing changes the length, folds across scripts or looks at its neighbors + let pairs = [ + ("İstanbul/Photo", "i̇stanbul"), // İ lower-cases to i + U+0307 + ("a/İ", "\u{307}"), // a match that starts inside an expansion + ("STRASSE", "straße"), // ß never equals ss, either way + ("Straße", "STRASSE"), + ("ẞ/x", "ß"), // capital sharp s lower-cases to ß + ("ΟΔΟΣ", "οδος"), // final sigma: Σ → ς at the end of a word + ("ΟΔΟΣ", "οδοσ"), + ("ΣΑΣ/ΟΔΟΣ", "σας"), + ("\u{212A}elvin", "kelvin"), // Kelvin sign → k + ("Dž/x", "dž"), // a title-case digraph + ("D:\\Фото\\Ё", "d:/фото/ё"), + (" Ünï ", "ünï"), // whitespace around the path is trimmed for is / startsWith / endsWith + ("Ünï", " ünï\t"), + ("\u{85}Ü\u{2003}", "ü"), + ("", "é"), + ("é", ""), + ]; + for (path, want) in pairs { + for op in OPS { + assert_eq!(fast(op, path, want), old(op, path, want), "{op} {path:?} {want:?}"); + } + } +} + +#[test] +fn a_path_or_text_longer_than_the_buffers_still_answers_as_before() { + // İ grows by half when lower-cased, so these cross FOLDED_PATH_BYTES and FOLDED_TEXT_BYTES on either side of the limit + for len in [FOLDED_PATH_BYTES / 3, FOLDED_PATH_BYTES / 2 - 1, FOLDED_PATH_BYTES / 2, FOLDED_PATH_BYTES, FOLDED_PATH_BYTES * 2] { + let path = format!("/Фото/{}/IMG.JPG", "İ".repeat(len / 2)); + for want_len in [3, FOLDED_TEXT_BYTES / 3, FOLDED_TEXT_BYTES / 2, FOLDED_TEXT_BYTES, FOLDED_TEXT_BYTES * 2] { + let tail = "i̇".repeat(want_len / 3); + for want in [tail.clone(), format!("{tail}/img.jpg"), format!("/фото/{tail}")] { + for op in OPS { + assert_eq!(fast(op, &path, &want), old(op, &path, &want), "{op} {len} {want_len}"); + } + } + } + } +} + +#[test] +fn a_text_that_is_not_a_string_still_answers_as_before() { + for value in [json!(7), json!(null), json!(true), json!(["é"])] { + for op in OPS { + let src = file("/Фото/7.jpg"); + assert_eq!(file_path_matches(op, &src, &value), file_path_matches_slow(op, "/Фото/7.jpg", &value), "{op} {value}"); + } + } +} + +/// Letters whose lower-casing is odd, plus plain ones, separators and whitespace. +fn glyph() -> impl Strategy { + prop::sample::select(vec![ + 'a', 'A', 's', 'S', 'ß', 'ẞ', 'İ', 'i', 'I', '\u{307}', 'é', 'É', 'ü', 'Ü', 'Σ', 'σ', 'ς', 'Dž', 'dž', 'DŽ', '\u{212A}', 'k', 'Ω', 'ω', 'Ё', + 'ё', 'Ф', 'ф', '日', '本', '/', '\\', ' ', '\t', '\u{85}', '\u{2003}', '.', '1', + ]) +} + +fn text() -> impl Strategy { + prop::collection::vec(glyph(), 0..14).prop_map(|v| v.into_iter().collect()) +} + +proptest! { + #![proptest_config(ProptestConfig::with_cases(4000))] + + #[test] + fn the_in_place_comparison_equals_the_allocating_one(path in text(), want in text(), op in prop::sample::select(OPS.to_vec())) { + prop_assert_eq!(fast(op, &path, &want), old(op, &path, &want), "{} {:?} {:?}", op, path, want); + } + + #[test] + fn a_slice_of_the_path_is_found_whatever_its_case(path in text(), from in 0usize..14, len in 0usize..14, upper in any::(), op in prop::sample::select(OPS.to_vec())) { + let chars: Vec = path.chars().collect(); + let from = from.min(chars.len()); + let to = (from + len).min(chars.len()); + let slice: String = chars[from..to].iter().collect(); + let want = if upper { slice.to_uppercase() } else { slice.to_lowercase() }; + prop_assert_eq!(fast(op, &path, &want), old(op, &path, &want), "{} {:?} {:?}", op, path, want); + } + + #[test] + fn any_unicode_text_agrees(path in "\\PC{0,12}", want in "\\PC{0,6}", op in prop::sample::select(OPS.to_vec())) { + prop_assert_eq!(fast(op, &path, &want), old(op, &path, &want), "{} {:?} {:?}", op, path, want); + } +} + +/// A timed loop over a library's worth of non-ASCII paths: run it with +/// `just test catalog path_fold_timing -- --ignored --nocapture` (release numbers: `--release`). +#[test] +#[ignore = "a measurement, not a check"] +fn path_fold_timing() { + const N: usize = 844_425; + let folders = ["Фото", "Ünïcode", "日本語", "Straße", "Éclair", "İstanbul"]; + let paths: Vec = (0..N).map(|i| file(&format!("/Volumes/share/{}/{}/IMG_{i:07}.JPG", folders[i % 6], folders[(i / 6) % 6]))).collect(); + let wants = [("contains", "ÉCLAIR/img_000"), ("contains", "tokyo"), ("startsWith", "/volumes/share/фото"), ("endsWith", "99.jpg")]; + for (op, want) in wants { + let value = Value::String(want.to_string()); + let t = std::time::Instant::now(); + let new_hits = paths.iter().filter(|p| file_path_matches(op, p, &value)).count(); + let new_time = t.elapsed(); + let t = std::time::Instant::now(); + let old_hits = paths.iter().filter(|p| matches!(p, Source::File { path } if file_path_matches_slow(op, path, &value))).count(); + let old_time = t.elapsed(); + assert_eq!(new_hits, old_hits); + println!("{op:<11} {want:<22} hits {new_hits:>7} in place {new_time:>10.2?} allocating {old_time:>10.2?}"); + } +} diff --git a/crates/catalog/tests/path_fold_heap.rs b/crates/catalog/tests/path_fold_heap.rs new file mode 100644 index 000000000..4fc1c8ef1 --- /dev/null +++ b/crates/catalog/tests/path_fold_heap.rs @@ -0,0 +1,59 @@ +//! Heap blocks a smart album's `filePath` rules allocate per photo, measured with dhat's testing mode. A rule over +//! a non-ASCII path or text (`crates/catalog/src/rules.rs`, `file_path_matches`) used to build two lower-cased +//! strings for every photo it was asked about; a large library asks it for every photo of every smart album, so +//! it compares in place. One test per binary (dhat's profiler is global). + +use lightcraft_catalog::{Catalog, Photo, PhotoId, RuleSet, Source}; +use serde_json::json; + +#[global_allocator] +static ALLOC: dhat::Alloc = dhat::Alloc; + +fn photo(id: u64, path: &str) -> Photo { + Photo::new(PhotoId(id), Source::File { path: path.to_string() }, "IMG_0001.JPG", "JPG", 6000, 4000, "2026-09-20T10:00:00") +} + +fn rule_set(op: &str, value: &str) -> RuleSet { + serde_json::from_value(json!({"rules": [{"field": "filePath", "op": op, "value": value}]})).unwrap() +} + +#[test] +fn a_file_path_rule_allocates_nothing_per_photo() { + let cat = Catalog::new(); + let photos: Vec = ["/Volumes/tokyo/Фото/Éclair/Straße/ÜBER_2026.JPG", "D:\\日本語\\İstanbul\\IMG_0001.jpg", "/srv/plain/ascii/path.jpg"] + .iter() + .zip(1u64..) + .map(|(p, i)| photo(i, p)) + .collect(); + let rules: Vec = [ + ("contains", "éCLAIR"), + ("contains", "tokyo"), + ("notContains", "ÜBER"), + ("is", "/srv/plain/ascii/path.jpg"), + ("isNot", "straße"), + ("startsWith", "\\Volumes\\TOKYO\\ФОТО"), + ("endsWith", ".JPG"), + ("isEmpty", ""), + ("isNotEmpty", ""), + ] + .iter() + .map(|(op, v)| rule_set(op, v)) + .collect(); + // warm anything lazily built (the clock, thread locals) before counting + let expected: Vec> = rules.iter().map(|r| photos.iter().map(|p| r.matches(p, &cat)).collect()).collect(); + let _profiler = dhat::Profiler::builder().testing().build(); + let before = dhat::HeapStats::get().total_blocks; + let mut hits = 0usize; + for _ in 0..100 { + for (r, want) in rules.iter().zip(&expected) { + for (p, w) in photos.iter().zip(want) { + let got = r.matches(p, &cat); + assert_eq!(got, *w); + hits += usize::from(got); + } + } + } + let blocks = dhat::HeapStats::get().total_blocks - before; + assert!(hits > 0, "the rules matched nothing: the test measures nothing"); + assert_eq!(blocks, 0, "{blocks} heap blocks for {} rule tests", 100 * rules.len() * photos.len()); +}