diff --git a/packages/python/e2e-attestations/bot-speed-walk-rows-1013.json b/packages/python/e2e-attestations/bot-speed-walk-rows-1013.json new file mode 100644 index 000000000..ffa86e507 --- /dev/null +++ b/packages/python/e2e-attestations/bot-speed-walk-rows-1013.json @@ -0,0 +1,7 @@ +{ + "command": "uv run python -m pytest tests/e2e/glob_speed_recursive_test.py tests/e2e/glob_speed_one_level_test.py tests/e2e/glob_speed_depth_bound_test.py tests/e2e/glob_speed_gitignore_test.py tests/e2e/glob_speed_dotfiles_test.py tests/e2e/glob_speed_matchers_test.py tests/e2e/glob_speed_node_modules_test.py tests/e2e/glob_speed_symlink_test.py tests/e2e/stat_speed_test.py tests/e2e/query_contract_test.py -q", + "ran_at": 1791930381, + "exit_code": 0, + "commit": "c06ef860f49ced8695614bd9e4bb36988aac0795", + "branch": "bot/speed-walk-rows-1013" +} diff --git a/packages/rust/changelog.d/2026-10-13-path-rows-built-on-walk-threads.md b/packages/rust/changelog.d/2026-10-13-path-rows-built-on-walk-threads.md new file mode 100644 index 000000000..d738a1a4c --- /dev/null +++ b/packages/rust/changelog.d/2026-10-13-path-rows-built-on-walk-threads.md @@ -0,0 +1,3 @@ +**Changed** + +- **A path table builds its rows on the walk threads, in smaller rows.** Recursive globs over large trees, such as `./**/*.md`, answer faster and use less memory. Output is unchanged. diff --git a/packages/rust/migrations.d/2026-10-13-path-rows-built-on-walk-threads.md b/packages/rust/migrations.d/2026-10-13-path-rows-built-on-walk-threads.md new file mode 100644 index 000000000..40875f004 --- /dev/null +++ b/packages/rust/migrations.d/2026-10-13-path-rows-built-on-walk-threads.md @@ -0,0 +1,25 @@ +### Faster recursive globs + +#### Summary + +A path table builds each row on the walk thread that found the file, and each row is smaller. Query results and their order are unchanged; nothing breaks. + +#### Required changes + +_None._ + +#### Deprecations removed + +_None._ + +#### Behavior changes without code changes + +_None._ + +#### Verification + +```bash +dirsql query "SELECT path FROM './**/*.md'" +``` + +Prints the same rows as before, sooner. diff --git a/packages/rust/src/scanner.rs b/packages/rust/src/scanner.rs index 1b152336d..46b0c4bca 100644 --- a/packages/rust/src/scanner.rs +++ b/packages/rust/src/scanner.rs @@ -82,9 +82,14 @@ pub fn scan_dirs(root: &Path, start: &Path, ignore: &TableMatcher) -> Vec Vec { + scan_glob_rows(root, glob, ignore, gitignore, check_root, &|rel_path| { + rel_path + }) +} + +/// [`scan_glob_checking_root`], each matched path made into `row` on the +/// thread that found it, so making rows overlaps the walk. +pub(crate) fn scan_glob_rows( + root: &Path, + glob: &PathGlob, + ignore: &TableMatcher, + gitignore: bool, + check_root: bool, + row: &(dyn Fn(PathBuf) -> T + Sync), +) -> Vec { if gitignore && check_root && !holds_git(root) && is_gitignored_path(root, true) { return Vec::new(); } @@ -201,7 +224,8 @@ pub fn scan_glob_checking_root( root, walker, &|rel_path| is_wanted(glob, ignore, rel_path), - &mut |rel_path, _| results.push(rel_path), + &|rel_path, _| row(rel_path), + &mut |found| results.push(found), ); results } @@ -476,12 +500,13 @@ pub(crate) fn parse_gitignore_arg(arg: &str) -> Result { /// `.git`, its `.gitignore` files apply. Symlinks are followed only as the /// glob allows, and not at all without one; a broken link or an unreadable /// directory contributes nothing. -fn walk( +fn walk( root: &Path, start: &Path, walker: Walk<'_>, keep: &(dyn Fn(&Path) -> bool + Sync), - visit: &mut dyn FnMut(PathBuf, &Path), + leaf: &(dyn Fn(PathBuf, &Path) -> L + Sync), + visit: &mut dyn FnMut(L), ) { let rel = start.strip_prefix(root).unwrap_or(start); let states = walker.glob.map_or_else(Vec::new, |glob| { @@ -498,10 +523,8 @@ fn walk( }; walk_in_order( Box::new(place), - &|place: Box>| (*place).explore(keep), - &mut |(rel, dir)| { - visit(rel, &dir); - }, + &|place: Box>| (*place).explore(keep, leaf), + visit, ); } @@ -516,12 +539,15 @@ struct Place<'a> { linked: bool, } -type Found = (PathBuf, Arc); - impl<'a> Place<'a> { - /// The directory's entries in walk order: each file `keep` takes, and - /// each directory to enter. - fn explore(self, keep: &(dyn Fn(&Path) -> bool + Sync)) -> Vec>, Found>> { + /// The directory's entries in walk order: each file `keep` takes, made + /// into a `leaf` with the directory it sits in, and each directory to + /// enter. + fn explore( + self, + keep: &(dyn Fn(&Path) -> bool + Sync), + leaf: &(dyn Fn(PathBuf, &Path) -> L + Sync), + ) -> Vec>, L>> { let Place { mut walk, dir, @@ -553,14 +579,11 @@ impl<'a> Place<'a> { Taken::Dir { .. } => true, }) }); - let dir: Arc = Arc::from(dir); - let mut steps: Vec>, Found>> = if walk.dirs { + let mut steps: Vec>, L>> = if walk.dirs { taken .iter() .filter_map(|taken| match taken { - Some(Taken::Dir { child, .. }) => { - Some(Step::Leaf((child.clone(), Arc::clone(&dir)))) - } + Some(Taken::Dir { child, .. }) => Some(Step::Leaf(leaf(child.clone(), &dir))), _ => None, }) .collect() @@ -599,7 +622,7 @@ impl<'a> Place<'a> { states: next, linked, }))), - Taken::File(child) => Some(Step::Leaf((child, Arc::clone(&dir)))), + Taken::File(child) => Some(Step::Leaf(leaf(child, &dir))), }); if steps.is_empty() { return entered.collect(); @@ -918,6 +941,7 @@ fn is_glob_match(glob: &PathGlob, rel_path: &Path) -> bool { mod tests { use super::*; use std::ffi::OsStr; + use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; #[test] fn scan_directory_reporting_walks_a_known_source_file() { @@ -1124,7 +1148,7 @@ mod tests { fn explored_dirs(place: Place<'_>) -> Vec> { place - .explore(&|_| true) + .explore(&|_| true, &|_, _| ()) .into_iter() .filter_map(|step| match step { Step::Dir(child) => Some(*child), @@ -1911,4 +1935,38 @@ mod tests { assert!(is_gitignored_path(&repo.path().join("debug.log"), false)); assert!(!is_gitignored_path(&repo.path().join("app.js"), false)); } + + #[test] + fn scan_glob_rows_makes_rows_on_the_walk_threads_in_scan_order() { + let dir = tempfile::tempdir().unwrap(); + for i in 0..64 { + let sub = dir.path().join(format!("d{i:02}")); + std::fs::create_dir(&sub).unwrap(); + std::fs::write(sub.join("f.md"), "").unwrap(); + } + let glob = compile_glob("**/*.md").unwrap(); + let none = TableMatcher::new(&[], &[]).unwrap(); + let in_flight = AtomicUsize::new(0); + let overlapped = AtomicBool::new(false); + + let rows = scan_glob_rows(dir.path(), &glob, &none, false, false, &|rel| { + if in_flight.fetch_add(1, Ordering::SeqCst) > 0 { + overlapped.store(true, Ordering::SeqCst); + } + for _ in 0..1_000_000 { + if overlapped.load(Ordering::SeqCst) { + break; + } + std::hint::spin_loop(); + } + in_flight.fetch_sub(1, Ordering::SeqCst); + rel + }); + + assert_eq!(rows, scan_glob(dir.path(), &glob, &none, false)); + assert!( + overlapped.load(Ordering::SeqCst), + "rows were made one at a time" + ); + } } diff --git a/packages/rust/src/vtab.rs b/packages/rust/src/vtab.rs index 93e4df151..16654032d 100644 --- a/packages/rust/src/vtab.rs +++ b/packages/rust/src/vtab.rs @@ -1,6 +1,7 @@ use std::ffi::c_int; use std::fs::{self, File}; use std::io::{self, Read}; +use std::num::NonZeroU32; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::{Arc, Mutex, OnceLock, Weak}; @@ -12,7 +13,7 @@ use rusqlite::vtab::Context; use rusqlite::{Connection, Result}; use crate::matcher::TableMatcher; -use crate::scanner::{PathGlob, scan_glob_checking_root, to_slash}; +use crate::scanner::{PathGlob, scan_glob_rows, to_slash}; use crate::vtab_scaffold::{self, TableSource}; pub use crate::vtab_scaffold::StatementScope; @@ -190,9 +191,9 @@ struct ScanSpec { /// so a row costs two allocations however many columns a query reads. struct FileRow { /// The path as scanned, root-relative, when `path` is not already it. - rel: Option, + rel: Option>, /// The path as reported, under the table's prefix when it has one. - path: String, + path: Box, spans: PathSpans, /// Stat'ed at most once per statement, and only when the statement reads /// a stat column or the content. @@ -209,9 +210,10 @@ impl FileRow { Ok(path) => (None, path), Err(rel) => { let path = reported_path(path_prefix, &rel); - (Some(rel), path) + (Some(rel.into_boxed_path()), path) } }; + let path = path.into_boxed_str(); let spans = PathSpans::of(&path); Self { rel, @@ -224,19 +226,21 @@ impl FileRow { /// The path as scanned, root-relative; where the lazy content read goes. fn rel(&self) -> &Path { - self.rel.as_deref().unwrap_or_else(|| Path::new(&self.path)) + self.rel + .as_deref() + .unwrap_or_else(|| Path::new(&*self.path)) } fn basename(&self) -> Option<&str> { - self.spans.name_at.map(|at| &self.path[at as usize..]) + self.spans.name_at.map(|at| &self.path[span_end(at)..]) } fn dir(&self) -> Option<&str> { - self.spans.dir_len.map(|len| &self.path[..len as usize]) + self.spans.dir_len.map(|len| &self.path[..span_end(len)]) } fn ext(&self) -> Option<&str> { - self.spans.ext_at.map(|at| &self.path[at as usize..]) + self.spans.ext_at.map(|at| &self.path[span_end(at)..]) } } @@ -244,18 +248,29 @@ impl FileRow { /// where [`Path`] has no answer: no basename or dir for an empty path, no ext /// for a dotfile or an extensionless name. [`Path::parent`] is a prefix of /// the path and [`Path::file_name`] and [`Path::extension`] are suffixes, so -/// one length each locates them. +/// one length each locates them. Each is held one past its offset, so an +/// absent span costs no more room than a present one. #[derive(Debug, PartialEq, Eq)] struct PathSpans { - name_at: Option, - dir_len: Option, - ext_at: Option, + name_at: Option, + dir_len: Option, + ext_at: Option, +} + +/// The offset a [`PathSpans`] span holds one past. +fn span_end(span: NonZeroU32) -> usize { + (span.get() - 1) as usize } impl PathSpans { fn of(path: &str) -> Self { let p = Path::new(path); - let span = |at: usize| u32::try_from(at).ok(); + let span = |at: usize| { + u32::try_from(at) + .ok()? + .checked_add(1) + .and_then(NonZeroU32::new) + }; let suffix_at = |s: &std::ffi::OsStr| span(path.len() - s.len()); Self { name_at: p.file_name().and_then(suffix_at), @@ -289,41 +304,6 @@ fn epoch_secs(time: Option) -> Option { i64::try_from(since.as_secs()).ok() } -/// The rows for the files a scan found, in scan order, unstat'ed. Built -/// across every core, since a large scan's paths each need splitting. -fn build_rows(path_prefix: &Path, rel_paths: Vec) -> Vec { - let workers = thread::available_parallelism().map_or(1, usize::from); - let per_worker = rel_paths.len().div_ceil(workers).max(1); - let row = |rel: PathBuf| FileRow::new(path_prefix, rel); - thread::scope(|scope| { - let handles: Vec<_> = chunks(rel_paths, per_worker) - .into_iter() - .map(|chunk| scope.spawn(move || chunk.into_iter().map(row).collect::>())) - .collect(); - handles - .into_iter() - .flat_map(|handle| handle.join().expect("a row builder only builds rows")) - .collect() - }) -} - -/// `items` cut into runs of `size` in order, the last run holding whatever -/// remains; one (possibly empty) run when there is less than a full one. -fn chunks(items: Vec, size: usize) -> Vec> { - let size = size.max(1); - let mut runs = Vec::with_capacity(items.len().div_ceil(size)); - let mut rest = items; - // Cut from the back so each item moves once; cutting from the front - // would move the whole remainder on every cut. - while rest.len() > size { - let start = (rest.len() - 1) / size * size; - runs.push(rest.split_off(start)); - } - runs.push(rest); - runs.reverse(); - runs -} - fn integer(n: Option) -> ValueRef<'static> { n.map_or(ValueRef::Null, ValueRef::Integer) } @@ -336,7 +316,7 @@ fn cell<'r>( facts: &dyn Fn(&FileRow) -> StatFacts, ) -> Option> { Some(match usize::try_from(i).unwrap_or(usize::MAX) { - PATH_COLUMN => ValueRef::from(row.path.as_str()), + PATH_COLUMN => ValueRef::from(&*row.path), BASENAME_COLUMN => ValueRef::from(row.basename()), DIR_COLUMN => ValueRef::from(row.dir()), EXT_COLUMN => ValueRef::from(row.ext()), @@ -440,14 +420,14 @@ impl TableSource for ScanSpec { fn rows(&self) -> Arc> { // The scan runs per statement rather than at CREATE, which is what // makes reads live: each statement sees the filesystem as it is now. - let rel_paths = scan_glob_checking_root( + let rows = Arc::new(scan_glob_rows( &self.root, &self.glob, &self.ignore, self.gitignore, self.check_root, - ); - let rows = Arc::new(build_rows(&self.path_prefix, rel_paths)); + &|rel| FileRow::new(&self.path_prefix, rel), + )); if let Ok(mut current) = self.current.lock() { *current = Arc::downgrade(&rows); } @@ -836,57 +816,17 @@ mod tests { } #[test] - fn chunks_cut_in_order_with_the_remainder_last() { - assert_eq!( - chunks(vec![1, 2, 3, 4, 5], 2), - vec![vec![1, 2], vec![3, 4], vec![5]] - ); - } - - #[test] - fn an_exact_multiple_cuts_into_full_runs() { - assert_eq!( - chunks(vec![1, 2, 3, 4, 5, 6], 3), - vec![vec![1, 2, 3], vec![4, 5, 6]] - ); - } - - #[test] - fn a_full_run_is_one_chunk() { - assert_eq!(chunks(vec![1, 2, 3], 3), vec![vec![1, 2, 3]]); - } - - #[test] - fn nothing_is_one_empty_chunk() { - assert_eq!(chunks(Vec::::new(), 4), vec![Vec::::new()]); - } - - #[test] - fn build_rows_keeps_scan_order() { - let rel_paths: Vec = (0..100) - .map(|n| PathBuf::from(format!("f{n:03}.md"))) - .collect(); - let rows = build_rows(Path::new("/root"), rel_paths); - assert_eq!(rows.len(), 100); - for (n, row) in rows.iter().enumerate() { - assert_eq!(row.path, format!("/root/f{n:03}.md")); - assert_eq!(row.rel(), Path::new(&format!("f{n:03}.md"))); - } - } - - #[test] - fn build_rows_stats_no_file() { - let rows = build_rows( - Path::new(""), - vec![PathBuf::from("a.md"), PathBuf::from("b.md")], - ); - assert!(rows.iter().all(|row| row.facts.get().is_none())); + fn a_row_reports_its_path_under_the_prefix_and_reads_from_the_scanned_one() { + let row = FileRow::new(Path::new("/root"), PathBuf::from("f001.md")); + assert_eq!(&*row.path, "/root/f001.md"); + assert_eq!(row.rel(), Path::new("f001.md")); } #[test] - fn build_rows_yields_nothing_for_an_empty_scan() { - let rows = build_rows(Path::new(""), Vec::new()); - assert!(rows.is_empty()); + fn a_new_row_is_unstatted_and_unread() { + let row = FileRow::new(Path::new(""), PathBuf::from("a.md")); + assert!(row.facts.get().is_none()); + assert!(row.content.get().is_none()); } #[test] @@ -1043,7 +983,7 @@ mod tests { .unwrap(); let reads = std::sync::Mutex::new(Vec::new()); let read = |row: &FileRow| { - reads.lock().unwrap().push(row.path.clone()); + reads.lock().unwrap().push(row.path.to_string()); Some(format!("body of {}", row.path)) }; @@ -1087,7 +1027,7 @@ mod tests { #[test] fn a_nested_path_splits_into_dir_basename_and_ext() { let row = row_for("", "nested/sub.txt"); - assert_eq!(row.path, "nested/sub.txt"); + assert_eq!(&*row.path, "nested/sub.txt"); assert_eq!(row.dir(), Some("nested")); assert_eq!(row.basename(), Some("sub.txt")); assert_eq!(row.ext(), Some("txt")); @@ -1103,7 +1043,7 @@ mod tests { #[test] fn an_absolute_row_reports_its_prefix_in_path_and_dir() { let row = row_for("/var/log", "a.log"); - assert_eq!(row.path, "/var/log/a.log"); + assert_eq!(&*row.path, "/var/log/a.log"); assert_eq!(row.dir(), Some("/var/log")); assert_eq!(row.basename(), Some("a.log")); assert_eq!( @@ -1184,7 +1124,7 @@ mod tests { #[test] fn a_row_not_yet_stat_or_read_stays_compact() { let size = std::mem::size_of::(); - assert!(size <= 112, "a FileRow is {size} bytes"); + assert!(size <= 80, "a FileRow is {size} bytes"); } #[test] fn a_content_column_reads_the_file_through_the_module() { diff --git a/packages/ts/e2e-attestations/bot-speed-walk-rows-1013.json b/packages/ts/e2e-attestations/bot-speed-walk-rows-1013.json new file mode 100644 index 000000000..bc1f21fd8 --- /dev/null +++ b/packages/ts/e2e-attestations/bot-speed-walk-rows-1013.json @@ -0,0 +1,7 @@ +{ + "command": "pnpm test:e2e", + "ran_at": 1791930449, + "exit_code": 0, + "commit": "4d053b2b310a7034ea244f217ed730973ac13a96", + "branch": "bot/speed-walk-rows-1013" +}