From 5bc2d53ad9249565d8c4719cc50b209a5837baa2 Mon Sep 17 00:00:00 2001 From: Jeremy Karst Date: Sun, 27 Sep 2026 14:19:39 -0400 Subject: [PATCH] Another performance pass: Batch writes and cut background work across indexing, search and the GUI --- crates/quicksearch-core/benches/index.rs | 79 ++++- .../quicksearch-core/benches/index_alloc.rs | 26 +- crates/quicksearch-core/benches/search.rs | 46 +-- crates/quicksearch-core/examples/walkprobe.rs | 29 +- crates/quicksearch-core/src/content.rs | 16 +- crates/quicksearch-core/src/coordinator.rs | 6 +- .../quicksearch-core/src/coordinator/inner.rs | 152 ++++++---- .../quicksearch-core/src/coordinator/tests.rs | 97 +++++- crates/quicksearch-core/src/db/repo.rs | 55 ++-- crates/quicksearch-core/src/db/repo_tests.rs | 53 +--- crates/quicksearch-core/src/db/schema.rs | 4 +- crates/quicksearch-core/src/extract/audio.rs | 18 ++ crates/quicksearch-core/src/extract/mod.rs | 28 +- crates/quicksearch-core/src/extract/office.rs | 125 ++++---- .../quicksearch-core/src/extract/plaintext.rs | 5 + .../src/file_handling/batch.rs | 252 ++++++---------- .../file_handling/count_and_extract_tests.rs | 3 +- .../quicksearch-core/src/file_handling/mod.rs | 4 +- .../src/file_handling/records.rs | 12 +- .../src/file_handling/tests.rs | 27 +- crates/quicksearch-core/src/incremental.rs | 178 +++++------ .../quicksearch-core/src/indexing/pipeline.rs | 23 +- crates/quicksearch-core/src/live.rs | 8 +- crates/quicksearch-core/src/query/pattern.rs | 74 ++--- crates/quicksearch-core/src/search/cascade.rs | 106 ++++--- .../src/search/cascade/passes.rs | 70 ++--- .../quicksearch-core/src/search/duplicates.rs | 28 ++ crates/quicksearch-core/src/search/fuzzy.rs | 161 +++++++--- crates/quicksearch-core/src/search/mod.rs | 7 +- crates/quicksearch-core/src/snippet.rs | 276 +++++++----------- crates/quicksearch-core/src/testutil.rs | 2 + crates/quicksearch-core/src/walk.rs | 160 +++++----- crates/quicksearch-core/src/walk/pool.rs | 9 +- crates/quicksearch-core/src/walk/tests.rs | 52 +++- crates/quicksearch-core/src/watcher.rs | 201 +++---------- crates/quicksearch-core/src/watcher_tests.rs | 140 +++------ crates/quicksearch-core/tests/common/mod.rs | 44 +-- crates/quicksearch-gui/src/app/status_bar.rs | 5 - crates/quicksearch-gui/src/hotkey/mod.rs | 136 +++++---- crates/quicksearch-gui/src/search_tab.rs | 1 - .../quicksearch-gui/src/search_tab/tests.rs | 6 +- 41 files changed, 1368 insertions(+), 1356 deletions(-) diff --git a/crates/quicksearch-core/benches/index.rs b/crates/quicksearch-core/benches/index.rs index 3ab714a..253f7b4 100644 --- a/crates/quicksearch-core/benches/index.rs +++ b/crates/quicksearch-core/benches/index.rs @@ -16,9 +16,8 @@ fn main() { divan::main(); } -/// Compressing an extracted document: pure CPU, done before the writer takes -/// the connection. `batch_*` measures a whole chunk, as `Bodies::fill` -/// compresses one. +/// Compressing an extracted document: pure CPU, done by the writer as each +/// row is stored. `batch_*` measures a whole walk batch of small bodies. mod zstd_encode { use super::*; @@ -38,9 +37,8 @@ mod zstd_encode { bencher.bench_local(move || enc.compress(divan::black_box(text)).unwrap()); } - /// One writer chunk of typical documents. A contention figure, not a - /// throughput one: the lock serializes the indexer against itself, and - /// all compression together is under 1% of a cold run. + /// One walk batch of typical documents. All compression together is + /// under 1% of a cold run. const BATCH: usize = 500; #[divan::bench] @@ -235,11 +233,9 @@ mod update_narrowing { } } -/// A watcher event for a directory whose files are all indexed and unchanged: -/// a re-sent `Create`, a `chmod -R`, the live view re-checking what it shows. -/// Nothing is written, so the cost is whatever it reads to learn that; the -/// full walk's answer is one `stat` per file. -mod unchanged_event { +/// A watcher event for a directory of indexed files, applied in one +/// transaction as the coordinator applies a slice of its queue. +mod directory_event { use super::*; use quicksearch_core::config::{Config, IgnoreSet}; use quicksearch_core::extract::Registry; @@ -251,8 +247,11 @@ mod unchanged_event { const FILES: usize = 500; + /// Every file unchanged: a re-sent `Create`, a `chmod -R`, the live view + /// re-checking what it shows. Nothing is written, so the cost is whatever + /// it reads to learn that; the full walk's answer is one `stat` per file. #[divan::bench] - fn directory_create(bencher: Bencher) { + fn unchanged(bencher: Bencher) { let (dir, db) = Scratch::db("bench-unchanged-event"); let tree = db.with_file_name("tree"); std::fs::create_dir_all(&tree).unwrap(); @@ -277,11 +276,65 @@ mod unchanged_event { cancel: &cancel, resume_from: 0, }; - apply_fs_event(&mut conn, &event, &config, &ignore, ®istry, &budget).unwrap() + // One transaction per event, as the coordinator's apply slice is. + let tx = conn.transaction().unwrap(); + let applied = + apply_fs_event(&tx, &event, &config, &ignore, ®istry, &budget).unwrap(); + tx.commit().unwrap(); + applied }; // Indexed once; every timed run finds it unchanged. apply(); bencher.bench_local(apply); drop(dir); } + + /// The same directory with every file's mtime moved on before each run, + /// so each event rewrites all of its rows and their text: a checkout or + /// an unpack, where commit and FTS5 segment costs add up per file. + #[divan::bench] + fn changed(bencher: Bencher) { + let (dir, db) = Scratch::db("bench-changed-event"); + let tree = db.with_file_name("tree"); + std::fs::create_dir_all(&tree).unwrap(); + let files: Vec = + (0..FILES).map(|i| tree.join(format!("f{:04}.txt", i))).collect(); + for file in &files { + std::fs::write(file, corpus::text(1 << 10, 4)).unwrap(); + } + let mut conn = + quicksearch_core::db::open_or_recreate(db.to_str().unwrap(), &Default::default()) + .unwrap(); + let config = Config::default(); + let ignore = IgnoreSet::compile(&config.indexing.ignore_patterns).unwrap(); + let registry = Registry::default_set(); + let event = FsEvent::Create(tree.clone()); + let cancel = AtomicBool::new(false); + let mut apply = move || { + let budget = Budget { + deadline: Instant::now() + Duration::from_secs(3600), + cancel: &cancel, + resume_from: 0, + }; + // One transaction per event, as the coordinator's apply slice is. + let tx = conn.transaction().unwrap(); + let applied = + apply_fs_event(&tx, &event, &config, &ignore, ®istry, &budget).unwrap(); + tx.commit().unwrap(); + applied + }; + apply(); + let mut tick = 0u64; + bencher + .with_inputs(|| { + tick += 1; + let at = std::time::UNIX_EPOCH + Duration::from_secs(1_700_000_000 + tick); + for file in &files { + let f = std::fs::File::options().write(true).open(file).unwrap(); + f.set_modified(at).unwrap(); + } + }) + .bench_local_values(|()| apply()); + drop(dir); + } } diff --git a/crates/quicksearch-core/benches/index_alloc.rs b/crates/quicksearch-core/benches/index_alloc.rs index 9764835..103a56a 100644 --- a/crates/quicksearch-core/benches/index_alloc.rs +++ b/crates/quicksearch-core/benches/index_alloc.rs @@ -157,12 +157,12 @@ fn walk_traffic_per_file(samples: &[corpus::Sample], config: &Config, registry: ); } -/// Rows per writer chunk: `batch::STORE_CHUNK`, the unit `Bodies::fill` is -/// handed. -const CHUNK: usize = 32; +/// Rows per writer turn: `pipeline::READY_TOPUP`, the most one +/// `store_extracted` call is handed. +const CHUNK: usize = 64; -/// The writer's half that runs outside the connection lock: one output -/// `Vec` per row against one arena per chunk. +/// The writer's compression: one output `Vec` per row against the run's +/// one encoder and its reused buffer. /// /// The zstd context does not appear here: `zstd::bulk::Compressor::new` /// allocates through zstd's own C allocator, so its cost is CPU only @@ -186,17 +186,15 @@ fn compression_traffic_per_chunk(config: &Config) { .map(|_| repo::encode_one(&doc).expect("encode")) .collect::>() }); - // What `Bodies::fill` does: one encoder and one arena for the chunk. - let arena = measure(|| { + // What `store_extracted` does: one encoder, its buffer reused per row. + let reused = measure(|| { let mut enc = DocEncoder::new().expect("encoder"); - let mut arena = Vec::new(); (0..CHUNK) - .map(|_| enc.encode_into(&doc, &mut arena).expect("encode")) - .collect::>() - .len() + .map(|_| enc.body(&doc).expect("encode").map_or(0, <[u8]>::len)) + .sum::() }); - for (shape, usage) in [("a Vec per row", per_row), ("one arena, chunk", arena)] { + for (shape, usage) in [("a Vec per row", per_row), ("one encoder, reused", reused)] { println!( "{:<24} {:>10} {:>10} {:>12} {:>12}", shape, @@ -207,8 +205,8 @@ fn compression_traffic_per_chunk(config: &Config) { ); } println!( - "\n The arena is reused across every chunk a writer call handles, so after the\n \ - first its growth is zero too. The zstd context is invisible here — see above.\n" + "\n The encoder lives for the whole run, so after the first turn its growth is\n \ + zero too. The zstd context is invisible here — see above.\n" ); } diff --git a/crates/quicksearch-core/benches/search.rs b/crates/quicksearch-core/benches/search.rs index 40d41bf..0e0152e 100644 --- a/crates/quicksearch-core/benches/search.rs +++ b/crates/quicksearch-core/benches/search.rs @@ -165,39 +165,40 @@ mod substring { bencher.bench(|| finder.find_iter(divan::black_box(text)).count()); } - /// What `pass_fulltext` runs per row on the literal path: a - /// case-sensitive count, then one folded extraction yielding both. + /// What `pass_fulltext` runs per row on the literal path: one folded + /// extraction yielding the window and both counts. #[divan::bench(args = corpus::SIZES)] fn cascade_row_sweeps(bencher: Bencher, size: usize) { - let pattern = literal(corpus::NEEDLE); let text = corpus::text_mixed(size, 4); let folded = corpus::text_folded(size, 4); - let opts = snippet::Options { approx_bytes: 600 }; + let term = corpus::NEEDLE.to_ascii_lowercase(); + let budget = 600; bencher.bench(|| { - let (s, b) = snippet::extract_folded( + snippet::extract_folded( divan::black_box(text), divan::black_box(folded), + &term, corpus::NEEDLE, - &opts, - ); - let a = pattern.count(text); - (a, b, s.ranges.len()) + budget, + ) + .map(|f| (f.count, f.exact, f.snippet.ranges.len())) }); } - /// The rejected shape: a separate folded count sweeps the document a - /// third time for a number the extraction already has. + /// The replaced shape: a separate case-sensitive count sweeps the + /// document a second time for a number the extraction now has. #[divan::bench(args = corpus::SIZES)] fn cascade_row_sweeps_separate_count(bencher: Bencher, size: usize) { - let pattern = literal(corpus::NEEDLE); let text = corpus::text_mixed(size, 4); let folded = corpus::text_folded(size, 4); - let opts = snippet::Options { approx_bytes: 600 }; + let term = corpus::NEEDLE.to_ascii_lowercase(); + let budget = 600; bencher.bench(|| { - let a = pattern.count(divan::black_box(text)); - let b = pattern.count_folded(divan::black_box(folded)); - let (s, _) = snippet::extract_folded(text, folded, corpus::NEEDLE, &opts); - (a, b, s.ranges.len()) + let exact = + memchr::memmem::find_iter(divan::black_box(text).as_bytes(), corpus::NEEDLE) + .count(); + let f = snippet::extract_folded(text, folded, &term, corpus::NEEDLE, budget); + (exact, f.map(|f| (f.count, f.snippet.ranges.len()))) }); } } @@ -211,13 +212,15 @@ mod snippet_extract { fn extract_folded(bencher: Bencher, size: usize) { let text = corpus::text_mixed(size, 64); let folded = corpus::text_folded(size, 64); - let opts = snippet::Options { approx_bytes: 600 }; + let term = corpus::NEEDLE.to_ascii_lowercase(); + let budget = 600; bencher.bench(|| { snippet::extract_folded( divan::black_box(text), divan::black_box(folded), + &term, corpus::NEEDLE, - &opts, + budget, ) }); } @@ -249,7 +252,6 @@ mod filename_ladder { #[divan::bench] fn find_first_ci_scratch(bencher: Bencher) { - let pattern = literal("quartzite"); let rows = corpus::rows(); let mut scratch = String::new(); bencher.bench_local(move || { @@ -258,14 +260,14 @@ mod filename_ladder { scratch.clear(); scratch.push_str(&row.name); scratch.make_ascii_lowercase(); - if pattern.find_first_folded(&scratch).is_some() { + if memchr::memmem::find(scratch.as_bytes(), b"quartzite").is_some() { found += 1; continue; } scratch.clear(); scratch.push_str(&row.path); scratch.make_ascii_lowercase(); - if pattern.find_first_folded(&scratch).is_some() { + if memchr::memmem::find(scratch.as_bytes(), b"quartzite").is_some() { found += 1; } } diff --git a/crates/quicksearch-core/examples/walkprobe.rs b/crates/quicksearch-core/examples/walkprobe.rs index 212a031..f99acc0 100644 --- a/crates/quicksearch-core/examples/walkprobe.rs +++ b/crates/quicksearch-core/examples/walkprobe.rs @@ -20,14 +20,11 @@ use std::sync::atomic::AtomicBool; use std::sync::Arc; -use std::time::{Instant, UNIX_EPOCH}; +use std::time::Instant; use quicksearch_core::config::{Config, IgnoreSet}; use quicksearch_core::extract::Registry; -use quicksearch_core::file_handling::{ - classify_for_indexing, filtered_walk, prepare_file_record, DirRows, FileIndexAction, - UnreadableDirs, -}; +use quicksearch_core::file_handling::{filtered_walk, prepare_file_record, UnreadableDirs}; use quicksearch_core::walk::{walk_indexable_files, WalkEvent}; fn main() { @@ -42,11 +39,10 @@ fn main() { let _ = std::fs::remove_file(&db); quicksearch_core::db::open_or_recreate(db.to_str().unwrap(), &config.processing) .expect("scratch index"); - let existing = DirRows::new(); let start = Instant::now(); let (seen, prepared) = match mode.as_str() { - "serial" => serial(&root, &config, &existing), + "serial" => serial(&root, &config), _ => parallel(&root, &config, db.to_str().unwrap()), }; let _ = std::fs::remove_file(&db); @@ -59,7 +55,8 @@ fn main() { ); } -fn serial(root: &str, config: &Config, existing: &DirRows) -> (usize, usize) { +/// Against the same empty index, so every file is new and gets prepared. +fn serial(root: &str, config: &Config) -> (usize, usize) { let ignore = IgnoreSet::compile(&[]).unwrap(); let registry = Registry::default_set(); // One for the whole walk, as a walk worker holds one. @@ -74,21 +71,7 @@ fn serial(root: &str, config: &Config, existing: &DirRows) -> (usize, usize) { let Ok(meta) = std::fs::metadata(entry.path()) else { continue; }; - let Some(mtime) = meta - .modified() - .ok() - .and_then(|t| t.duration_since(UNIX_EPOCH).ok()) - .map(|d| d.as_secs()) - else { - continue; - }; - let name = std::path::Path::new(&path) - .file_name() - .map(|n| n.to_string_lossy().into_owned()) - .unwrap_or_default(); - if classify_for_indexing(&name, mtime, existing) != FileIndexAction::Skip - && prepare_file_record(&path, &meta, config, ®istry, &mut scratch).is_some() - { + if prepare_file_record(&path, &meta, config, ®istry, &mut scratch).is_some() { prepared += 1; } } diff --git a/crates/quicksearch-core/src/content.rs b/crates/quicksearch-core/src/content.rs index c2fbc95..9d1eb10 100644 --- a/crates/quicksearch-core/src/content.rs +++ b/crates/quicksearch-core/src/content.rs @@ -331,16 +331,14 @@ mod tests { use crate::db::open_or_recreate; use crate::db::repo::{self, insert_file, NewFile}; - use crate::file_handling::{ - extract_scope_prepare, store_extracted, Bodies, ExtractScope, Stored, - }; + use crate::file_handling::{extract_scope_prepare, store_extracted, ExtractScope, Stored}; use crate::mime::FileType; use crate::testutil::Scratch; use std::path::PathBuf; use std::time::{Duration, Instant}; - fn bodies() -> Bodies { - Bodies::new().unwrap() + fn encoder() -> crate::db::repo::DocEncoder { + crate::db::repo::DocEncoder::new().unwrap() } fn tree(s: &Scratch) -> PathBuf { @@ -428,7 +426,7 @@ mod tests { assert!(pass.finish()); let stop = Arc::new(AtomicBool::new(false)); let far = Instant::now() + Duration::from_secs(60); - store_extracted(conn, &rows, &stop, far, &mut bodies()).unwrap() + store_extracted(conn, &rows, &stop, far, &mut encoder()).unwrap() } #[test] @@ -552,7 +550,7 @@ mod tests { // A deadline already gone by: one row, then out. let past = Instant::now() - Duration::from_secs(1); assert_eq!( - store_extracted(&conn_mutex, &rows, &stop, past, &mut bodies()).unwrap(), + store_extracted(&conn_mutex, &rows, &stop, past, &mut encoder()).unwrap(), Stored { consumed: 1, written: 1 @@ -560,7 +558,7 @@ mod tests { ); let far = Instant::now() + Duration::from_secs(60); assert_eq!( - store_extracted(&conn_mutex, &rows[1..], &stop, far, &mut bodies()).unwrap(), + store_extracted(&conn_mutex, &rows[1..], &stop, far, &mut encoder()).unwrap(), Stored { consumed: 4, written: 4 @@ -580,7 +578,7 @@ mod tests { // Stopped before it starts: nothing consumed. stop.store(true, std::sync::atomic::Ordering::Relaxed); assert_eq!( - store_extracted(&conn_mutex, &rows, &stop, far, &mut bodies()).unwrap(), + store_extracted(&conn_mutex, &rows, &stop, far, &mut encoder()).unwrap(), Stored::default() ); } diff --git a/crates/quicksearch-core/src/coordinator.rs b/crates/quicksearch-core/src/coordinator.rs index 9a17921..50b87f8 100644 --- a/crates/quicksearch-core/src/coordinator.rs +++ b/crates/quicksearch-core/src/coordinator.rs @@ -43,8 +43,6 @@ use inner::Inner; /// Pending-event ceiling; beyond this a full run is cheaper than replay. const PENDING_OVERFLOW: usize = 100_000; -const FILE_COUNT_INTERVAL: Duration = Duration::from_secs(30); - #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum IndexMode { Auto, @@ -211,7 +209,7 @@ impl IndexCoordinator { watcher_rx: None, watcher_gen: 0, pending: inner::Debounced::default(), - targeted: HashMap::new(), + targeted: inner::Debounced::default(), resume_from: HashMap::new(), needs_full_run: false, run_retry: inner::Backoff::default(), @@ -219,7 +217,7 @@ impl IndexCoordinator { reconcile_done: None, reconcile_cut_short: false, run_in_flight: false, - files_at: None, + files_stale: true, was_busy: false, write_conn: None, ignore: Arc::new(IgnoreSet::compile(&[]).expect("empty ignore set")), diff --git a/crates/quicksearch-core/src/coordinator/inner.rs b/crates/quicksearch-core/src/coordinator/inner.rs index deef040..b45b2a8 100644 --- a/crates/quicksearch-core/src/coordinator/inner.rs +++ b/crates/quicksearch-core/src/coordinator/inner.rs @@ -8,9 +8,10 @@ const RUN_RETRY_BASE: Duration = Duration::from_secs(30); const RUN_RETRY_MAX: Duration = Duration::from_secs(300); -/// The watcher's event queue and the two timers that decide when to apply -/// it: once no event has arrived for `pending_settle`, or once the oldest -/// has waited `pending_max_defer`, so a trickle cannot defer it forever. +/// An event queue and the two timers that decide when to apply it: once no +/// event has arrived for `pending_settle`, or once the oldest has waited +/// `pending_max_defer`, so a trickle cannot defer it forever. One for the +/// watcher's events, one for the paths a frontend is displaying. #[derive(Default)] pub(super) struct Debounced { pub(super) events: HashMap, @@ -107,6 +108,17 @@ impl Backoff { } } +/// Whether `new` changes anything [`Inner::start_watcher`] registers with. +/// Every other edit (theme, shortcut, search limits) keeps the running +/// watcher: restarting it re-walks every directory under every root to +/// re-register its watch, and misses whatever changes in the gap. +pub(super) fn watch_inputs_differ(old: &Config, new: &Config) -> bool { + old.resolved_indexing_paths() != new.resolved_indexing_paths() + || old.indexing.include_hidden != new.indexing.include_hidden + || old.indexing.follow_symlinks != new.indexing.follow_symlinks + || old.indexing.ignore_patterns != new.indexing.ignore_patterns +} + /// Log a failed apply of `what` by the queue's policy: a targeted failure /// only logs, a pending one also schedules a full run. `true` for the latter. fn apply_failed(targeted: bool, what: &str, e: impl std::fmt::Display) -> bool { @@ -149,7 +161,7 @@ pub(super) struct Inner { /// Paths a frontend asked for by name ([`IndexCoordinator::update_paths`]). /// Survives [`Inner::clear_pending`] and applies in manual mode: it keeps /// the rows a user is *reading* in step with the disk. - pub(super) targeted: HashMap, + pub(super) targeted: Debounced, /// How far a directory event got before its turn's budget ran out. pub(super) resume_from: HashMap, pub(super) needs_full_run: bool, @@ -163,7 +175,9 @@ pub(super) struct Inner { /// Set while a full run holds the database; cleared by the tick that sees /// it end. pub(super) run_in_flight: bool, - pub(super) files_at: Option, + /// The published row count may be behind the database: set by anything + /// that could have written, cleared by [`Inner::refresh_file_count`]. + pub(super) files_stale: bool, /// Drives [`Inner::go_idle`] once per busy→idle transition. pub(super) was_busy: bool, pub(super) write_conn: Option, @@ -207,13 +221,14 @@ impl Inner { CoordCmd::ConfigChanged(new) => { let want_auto = new.indexing.auto_index; let actions = diff_actions(&self.config, &new); + let rewatch = self.watcher.is_none() || watch_inputs_differ(&self.config, &new); self.config = new; if let Err(e) = self.reload_filters() { crate::log_warn!("coordinator: {}", e); } // The write connection may point at an old database_path. self.write_conn = None; - self.files_at = None; + self.files_stale = true; if !actions.requires_rebuild && !actions.work.is_empty() { self.start_work(actions.work); } @@ -221,7 +236,8 @@ impl Inner { (true, false) => self.enter_auto(), (false, true) => self.enter_manual_stopped(), // Still automatic: the watcher follows the new config. - (true, true) => self.start_watcher(), + (true, true) if rewatch => self.start_watcher(), + (true, true) => {} (false, false) => {} } self.refresh_last_full_index(); @@ -229,7 +245,7 @@ impl Inner { CoordCmd::RebuildIndex => { let db = self.db_path(); self.write_conn = None; - self.files_at = None; + self.files_stale = true; self.pending_work = None; self.reconcile_done = None; if let Err(e) = self.indexing.delete_index_for_rebuild(&db) { @@ -260,7 +276,7 @@ impl Inner { shared.files = Some(0); shared.root_counts = Arc::new(Vec::new()); drop(shared); - self.files_at = None; + self.files_stale = true; } CoordCmd::UpdatePaths(paths) => { // Only paths under an indexed root: nothing downstream checks, @@ -275,11 +291,8 @@ impl Inner { let spelled = path.to_string_lossy(); prefixes.iter().any(|lo| spelled.starts_with(lo.as_str())) }; - paths - .into_iter() - .filter(under_root) - .filter_map(verb_for) - .for_each(|event| enqueue(&mut self.targeted, event)); + self.targeted + .push(paths.into_iter().filter(under_root).filter_map(verb_for)); self.was_busy = true; } CoordCmd::Shutdown => unreachable!("handled in run()"), @@ -313,7 +326,7 @@ impl Inner { self.defer_runs(); } self.refresh_last_full_index(); - self.files_at = None; + self.files_stale = true; if self.mode == IndexMode::ManualRunning { self.mode = IndexMode::ManualStopped; } @@ -321,8 +334,10 @@ impl Inner { self.refresh_file_count(); - // Ahead of every gate: rows a user is looking at right now. - if !self.targeted.is_empty() { + // Ahead of every gate: rows a user is looking at right now. Settled + // like the watcher's queue, or a growing file on screen, re-sent on + // every live refresh, is re-read, hashed and re-indexed every tick. + if !self.targeted.events.is_empty() && self.targeted.settled(&self.watcher_config) { self.apply_targeted(); } @@ -369,6 +384,8 @@ impl Inner { return; } self.was_busy = false; + // Whatever made it busy may have added or removed rows. + self.files_stale = true; self.write_conn = None; crate::platform::release_free_heap(); } @@ -486,7 +503,7 @@ impl Inner { self.pending.clear(); // Resume points for `targeted` survive: that queue outlives this call. self.resume_from - .retain(|p, _| self.targeted.contains_key(p)); + .retain(|p, _| self.targeted.events.contains_key(p)); } /// Apply as much of the queue as fits in [`APPLY_BUDGET`], removals first: @@ -510,49 +527,64 @@ impl Inner { self.pending.after_partial_apply(); } - /// Drain one queue's removals, then its upserts, within [`APPLY_BUDGET`]. - /// `targeted` picks the queue and the failure policy: pending failures - /// escalate to [`Inner::needs_full_run`], targeted ones only log. + /// Drain one queue's removals, then its upserts, within [`APPLY_BUDGET`], + /// in one transaction. `targeted` picks the queue and the failure policy: + /// pending failures escalate to [`Inner::needs_full_run`], targeted ones + /// only log. Measured against a transaction per file (`benches/index.rs`, + /// `directory_event`, one event over 500 small text files): 250 → + /// 88 ms with every file changed, 1.55 → 1.09 ms with none. fn apply_queue(&mut self, conn: &mut Connection, targeted: bool) { let deadline = Instant::now() + APPLY_BUDGET; let chunk = self.config.processing.batch_size.max(1); + let tx = match conn.transaction() { + Ok(tx) => tx, + Err(e) => { + self.needs_full_run |= apply_failed(targeted, "begin", e); + return; + } + }; let queue = if targeted { - &mut self.targeted + &mut self.targeted.events } else { &mut self.pending.events }; - let removals: Vec = queue - .iter() - .filter(|(_, ev)| is_removal(ev)) - .map(|(p, _)| p.clone()) - .collect(); + // Moved out, not cloned: what the budget leaves goes back below, so a + // storm's queue is not copied whole on every slice, and the map it + // leaves behind holds only what is left. Measured (200k targeted + // removals): drained in 10.0 → 9.1 s, peak RSS 85 → 81 MB. + let (mut removals, mut upserts) = (Vec::new(), Vec::new()); + for (path, ev) in std::mem::take(queue) { + if is_removal(&ev) { + removals.push(path); + } else { + upserts.push((path, ev)); + } + } + + let mut removed = 0; for batch in removals.chunks(chunk) { - if let Err(e) = crate::incremental::remove_paths(conn, batch, chunk) { + if let Err(e) = crate::incremental::remove_paths(&tx, batch) { // The batch leaves the queue either way: retrying a failed // write every tick is worse, and the full run recovers the rows. self.needs_full_run |= apply_failed(targeted, "remove", e); } - for path in batch { - queue.remove(path); - } + removed += batch.len(); if Instant::now() >= deadline { break; } } + queue.extend( + removals + .drain(removed..) + .map(|path| (path.clone(), FsEvent::Remove(path))), + ); + let mut upserts = upserts.into_iter(); if Instant::now() < deadline { - let upserts: Vec = queue - .iter() - .filter(|(_, ev)| !is_removal(ev)) - .map(|(p, _)| p.clone()) - .collect(); - for path in upserts { - let Some(ev) = queue.remove(&path) else { - continue; - }; + for (path, ev) in upserts.by_ref() { match apply_fs_event( - conn, + &tx, &ev, &self.config, &self.ignore, @@ -582,6 +614,10 @@ impl Inner { } } } + queue.extend(upserts); + if let Err(e) = tx.commit() { + self.needs_full_run |= apply_failed(targeted, "commit", e); + } } /// Apply the by-name queue: the paths a frontend is displaying. Never @@ -603,6 +639,7 @@ impl Inner { }; self.apply_queue(&mut conn, true); self.write_conn = Some(conn); + self.targeted.after_partial_apply(); } fn ensure_write_conn(&mut self) -> Result { @@ -684,8 +721,9 @@ impl Inner { // target (`aliased_paths`), and those rows would leak until a rebuild. self.pending.keep_removals(); // A stale resume point would `skip` entries of an unrelated walk. - self.resume_from - .retain(|p, _| self.pending.events.contains_key(p) || self.targeted.contains_key(p)); + self.resume_from.retain(|p, _| { + self.pending.events.contains_key(p) || self.targeted.events.contains_key(p) + }); if let Err(e) = self .indexing .start_indexing(roots, self.db_path(), self.config.clone()) @@ -864,8 +902,16 @@ impl Inner { self.set_watcher_status(WatcherStatus::Off); } + /// Publish `status`, waking the frontend when it changed: registration + /// can finish minutes after startup with no input to repaint on, and a + /// frontend polling for it would repaint the whole time. fn set_watcher_status(&self, status: WatcherStatus) { - crate::lock_ok(&self.shared).watcher = status; + let mut shared = crate::lock_ok(&self.shared); + if shared.watcher != status { + shared.watcher = status; + drop(shared); + (self.notify)(); + } } fn db_path(&self) -> String { @@ -883,20 +929,18 @@ impl Inner { Ok(()) } - /// Re-read the published row count, at most every [`FILE_COUNT_INTERVAL`]. + /// Re-read the published row count once something may have changed it. /// Called only from the idle half of [`Inner::tick`], so it cannot run /// while a full run holds the database; `COUNT(*)` is still a key scan, - /// hence the interval and the interrupt guard. + /// hence the interrupt guard. It used to run every 30 s whatever the + /// mode, opening a connection each time to recount an unchanged index. fn refresh_file_count(&mut self) { - if self - .files_at - .is_some_and(|at| at.elapsed() < FILE_COUNT_INTERVAL) - { + if !self.files_stale { return; } - // Stamped before the read: a failing count must back off like a - // successful one, or a missing index means an open attempt every tick. - self.files_at = Some(Instant::now()); + // Cleared before the read: a failing count must not retry every + // tick, or a missing index means an open attempt every second. + self.files_stale = false; let Ok(conn) = db::open_existing(&self.db_path(), false) else { return; @@ -962,7 +1006,7 @@ impl Inner { let busy = reconcile.is_some() || self.indexing.is_active(); let mut shared = crate::lock_ok(&self.shared); shared.mode = self.mode; - shared.queued_events = self.pending.events.len() + self.targeted.len(); + shared.queued_events = self.pending.events.len() + self.targeted.events.len(); shared.reconcile = reconcile; drop(shared); diff --git a/crates/quicksearch-core/src/coordinator/tests.rs b/crates/quicksearch-core/src/coordinator/tests.rs index f893eb5..a93ca1d 100644 --- a/crates/quicksearch-core/src/coordinator/tests.rs +++ b/crates/quicksearch-core/src/coordinator/tests.rs @@ -350,12 +350,10 @@ fn stopping_cancels_a_queued_walk_but_not_a_queued_prune() { coord.shutdown(); } -/// Short debounce windows so trailing-edge events flush within test -/// timeouts (production defaults are 30 s / 2 s). +/// Short debounce windows so trailing-edge events apply within test +/// timeouts (production defaults are 2 s / 30 s). fn fast_watcher() -> WatcherConfig { WatcherConfig { - throttle_window: Duration::from_millis(300), - tick_interval: Duration::from_millis(100), pending_settle: Duration::from_millis(200), pending_max_defer: Duration::from_secs(3), ..WatcherConfig::default() @@ -473,6 +471,38 @@ fn update_paths_indexes_one_file_with_indexing_stopped() { coord.shutdown(); } +/// A path re-sent on every live refresh, as a growing file on screen is, +/// waits for the queue to settle instead of being re-indexed every tick. +#[test] +fn update_paths_waits_for_the_queue_to_settle() { + let f = Fixture::new(false); + std::fs::write(f.dir.join("seed.txt"), "initial content").unwrap(); + f.seed_index(); + let settle = WatcherConfig { + pending_settle: Duration::from_secs(1), + pending_max_defer: Duration::from_secs(20), + ..WatcherConfig::default() + }; + let coord = start_coord_watching(f.config.clone(), settle); + let added = f.dir.join("growing.log"); + std::fs::write(&added, "first line").unwrap(); + + let spam_until = std::time::Instant::now() + Duration::from_secs(3); + while std::time::Instant::now() < spam_until { + coord.update_paths(vec![added.clone()]); + std::thread::sleep(Duration::from_millis(100)); + } + assert_eq!( + f.stored_mtime(&added), + None, + "applied while the path was still being re-sent" + ); + wait_for("the settled insert", Duration::from_secs(20), || { + f.stored_mtime(&added).is_some() + }); + coord.shutdown(); +} + #[test] fn update_paths_leaves_a_row_that_already_agrees_alone() { let f = Fixture::new(false); @@ -582,6 +612,38 @@ fn auto_mode_runs_initial_index_and_applies_watcher_events() { coord.shutdown(); } +/// The published count follows the coordinator's own writes, and an idle +/// coordinator does not go back to recount: a row slipped in behind its back +/// stays unreported until something it does changes the index. +#[test] +fn the_file_count_is_refreshed_by_writes_not_by_time() { + let f = Fixture::new(true); + std::fs::write(f.dir.join("seed.txt"), "initial content").unwrap(); + let coord = start_coord_watching(f.config.clone(), fast_watcher()); + wait_for("initial auto index", Duration::from_secs(20), || { + coord.state().files == Some(1) + }); + + std::fs::write(f.dir.join("later.txt"), "arrived later").unwrap(); + wait_for( + "the count to follow an incremental add", + Duration::from_secs(20), + || coord.state().files == Some(2), + ); + + let conn = db::open_existing(&f.db.to_string_lossy(), true).unwrap(); + conn.execute( + "INSERT INTO files (name, parent, size, mtime, type, content_state) + VALUES ('ghost.txt', '/nowhere/', 0, 0, 0, 3)", + [], + ) + .unwrap(); + drop(conn); + std::thread::sleep(Duration::from_secs(3)); + assert_eq!(coord.state().files, Some(2), "an idle tick recounted"); + coord.shutdown(); +} + #[test] fn auto_mode_reports_an_active_watcher() { let f = Fixture::new(true); @@ -882,6 +944,33 @@ fn an_unreadable_path_is_not_treated_as_deleted() { std::fs::remove_file(&present).ok(); } +/// Only what the watcher registers with restarts it; a theme or search +/// setting must leave a large tree's watches alone. +#[test] +fn only_watch_inputs_restart_the_watcher() { + let old = Config::default(); + let differs = |edit: fn(&mut Config)| { + let mut new = old.clone(); + edit(&mut new); + inner::watch_inputs_differ(&old, &new) + }; + assert!(!differs(|_| {})); + assert!(!differs(|c| c.search.debounce_ms += 100)); + assert!(!differs(|c| c.processing.batch_size += 1)); + assert!(!differs(|c| c.indexing.reindex_interval_minutes += 1)); + assert!(differs(|c| c + .paths + .indexing_paths + .push("/elsewhere".into()))); + assert!(differs( + |c| c.indexing.include_hidden = !c.indexing.include_hidden + )); + assert!(differs( + |c| c.indexing.follow_symlinks = !c.indexing.follow_symlinks + )); + assert!(differs(|c| c.indexing.ignore_patterns.push("*.tmp".into()))); +} + #[test] fn enqueue_last_wins_and_rename_splits() { let mut pending = HashMap::new(); diff --git a/crates/quicksearch-core/src/db/repo.rs b/crates/quicksearch-core/src/db/repo.rs index a1a4cbf..fc63ae0 100644 --- a/crates/quicksearch-core/src/db/repo.rs +++ b/crates/quicksearch-core/src/db/repo.rs @@ -275,13 +275,11 @@ impl DocDecoder { const ZSTD_LEVEL: i32 = 3; /// Reusable compression context for the `documents_text` sidecar; one per -/// batch. Measured (`benches/index.rs`, group `zstd_encode`). +/// indexing run. Measured (`benches/index.rs`, group `zstd_encode`). pub struct DocEncoder { ctx: zstd::bulk::Compressor<'static>, - /// One row's compressed output, reused. `zstd`'s `WriteBuf` for `Vec` - /// writes from offset 0 and sets the length, so a body cannot be - /// compressed straight into a shared arena; it lands here and is copied - /// across, with no allocation once both buffers have grown. + /// The last row's compressed output, reused, so the writer allocates + /// nothing per document once it has grown. row: Vec, } @@ -300,14 +298,12 @@ impl DocEncoder { .map_err(|e| format!("zstd encode: {}", e)) } - /// Append the compressed form of `text` to `arena`, returning where it - /// landed, so a whole batch's bodies share one allocation instead of one - /// `Vec` per document. - pub fn encode_into( - &mut self, - text: &str, - arena: &mut Vec, - ) -> Result, String> { + /// The compressed form of `text`, valid until the next call; `None` for an + /// empty body, which gets no `documents_text` row. + pub fn body(&mut self, text: &str) -> Result, String> { + if text.is_empty() { + return Ok(None); + } self.row.clear(); // `compress_to_buffer` writes into the buffer's capacity and fails // rather than growing it, so the room has to be there first. @@ -315,9 +311,7 @@ impl DocEncoder { self.ctx .compress_to_buffer(text.as_bytes(), &mut self.row) .map_err(|e| format!("zstd encode: {}", e))?; - let start = arena.len(); - arena.extend_from_slice(&self.row); - Ok(start..arena.len()) + Ok(Some(&self.row)) } } @@ -739,10 +733,16 @@ pub fn rows_in_range_page( /// Reset every failed file to pending and drop the failure records, so the /// coming run re-attempts them. Returns how many rows were reset. +/// +/// Found through `failed_files`, which holds exactly the failed rows (see +/// [`STATE_FAILED`]): no index covers `content_state = 2`, so asking `files` +/// scanned the table every run. Measured at 373k rows, 287 failed: 16 → +/// 0.3 ms with the table cached. pub fn retry_failed_files(tx: &Transaction<'_>) -> Result { let reset = exec( tx, - "UPDATE files SET content_state = ?1 WHERE content_state = ?2", + "UPDATE files SET content_state = ?1 \ + WHERE id IN (SELECT file_id FROM failed_files) AND content_state = ?2", params![STATE_PENDING, STATE_FAILED], || "reset failed files to pending".to_string(), )?; @@ -895,9 +895,22 @@ pub fn analyzed_file_count(conn: &Connection) -> Option { rows.max() } -/// Checkpoint, VACUUM, `PRAGMA optimize`, checkpoint; the trailing checkpoint -/// empties what VACUUM's copy-back and `optimize` wrote to the log. Returns -/// whether it vacuumed. +/// Create `idx_files_hash` if it is missing: after a fresh index's first run, +/// in one sorted build. It is not in [`crate::db::schema::SCHEMA_CURRENT`]: +/// a SHA-256 key lands on a random leaf, so kept up through a bulk load, every +/// walk commit rewrote hundreds of the index's pages. Measured (`memprobe +/// cold`): bytes written 4.77 → 2.87 GB over `/usr` (373k files), 938 → 855 +/// MiB over the cargo registry (41k), CPU within noise, and about 10 MiB more +/// peak for the sort. Only [`crate::search::find_duplicate_groups`] reads it, +/// and until the build it sorts the table instead. +pub fn build_hash_index(conn: &Connection) -> Result<(), String> { + conn.execute_batch("CREATE INDEX IF NOT EXISTS idx_files_hash ON files(hash);") + .map_err(|e| format!("build hash index: {}", e)) +} + +/// Checkpoint, [`build_hash_index`], VACUUM, `PRAGMA optimize`, checkpoint; +/// the trailing checkpoint empties what VACUUM's copy-back and `optimize` +/// wrote to the log. Returns whether it vacuumed. /// /// Run on a connection from [`crate::db::open::open_maintenance`], never the /// indexer's. VACUUM's temporary database goes in the directory of `db_path`, @@ -922,6 +935,8 @@ pub fn maintain(conn: &Connection, db_path: &str) -> Result { #[cfg(feature = "probe")] probe.step("maintain checkpoint"); + build_hash_index(conn)?; + let page_count = pragma_number(conn, "page_count")?; let freelist = pragma_number(conn, "freelist_count")?; diff --git a/crates/quicksearch-core/src/db/repo_tests.rs b/crates/quicksearch-core/src/db/repo_tests.rs index 1a05033..6a2df7e 100644 --- a/crates/quicksearch-core/src/db/repo_tests.rs +++ b/crates/quicksearch-core/src/db/repo_tests.rs @@ -224,29 +224,18 @@ fn a_compressed_body_carries_its_uncompressed_length() { } } -/// Several rows into one arena, as the writer does. `zstd`'s `WriteBuf` for -/// `Vec` writes from offset zero and sets the length, so compressing straight -/// into a shared arena would overwrite the previous row. Each body must come -/// back byte-identical out of its own range. +/// Several rows through one encoder, as the writer does: each body must come +/// back byte-identical from the slice handed out for it, whatever the size +/// of the rows before it, and an empty body gets none. #[test] -fn an_arena_keeps_every_row_it_is_given() { +fn one_encoder_serves_every_row_it_is_given() { let mut enc = DocEncoder::new().unwrap(); - let bodies = [ - "the first document", - "", - "a considerably longer second document ".repeat(512).as_str(), - "third", - ] - .map(str::to_string); - - let mut arena = Vec::new(); - let mut ranges = Vec::new(); - for text in &bodies { - ranges.push(enc.encode_into(text, &mut arena).unwrap()); - } - - for (text, at) in bodies.iter().zip(&ranges) { - let blob = &arena[at.clone()]; + let long = "a considerably longer second document ".repeat(512); + for text in ["the first document", "", long.as_str(), "third"] { + let Some(blob) = enc.body(text).unwrap() else { + assert!(text.is_empty(), "only an empty body has no blob"); + continue; + }; assert_eq!( raw_text_len(blob), Some(text.len() as u64), @@ -254,28 +243,10 @@ fn an_arena_keeps_every_row_it_is_given() { ); assert_eq!( DocDecoder::new().unwrap().decode(blob), - Some(text.as_str()), - "a row did not survive sharing the arena" + Some(text), + "a row did not survive the reused buffer" ); } - - // The ranges tile the arena in order: a gap or an overlap means one row - // landed on another. - let mut next = 0; - for at in &ranges { - assert_eq!(at.start, next, "rows must be contiguous"); - next = at.end; - } - assert_eq!(next, arena.len(), "the arena holds exactly the four bodies"); - - // Reuse: a second batch must not read the first one's bytes. - arena.clear(); - let at = enc.encode_into("a fresh batch", &mut arena).unwrap(); - assert_eq!(at.start, 0); - assert_eq!( - DocDecoder::new().unwrap().decode(&arena[at]), - Some("a fresh batch") - ); } #[test] diff --git a/crates/quicksearch-core/src/db/schema.rs b/crates/quicksearch-core/src/db/schema.rs index c6d6091..1fcd377 100644 --- a/crates/quicksearch-core/src/db/schema.rs +++ b/crates/quicksearch-core/src/db/schema.rs @@ -177,6 +177,9 @@ pub const PRAGMAS_WALK_READER: &str = " /// filename query already does: `searchtime` at 300k rows, 25 ms with or /// without a filter, where the indexes had made a one-day filter 0.06 ms. /// +/// `idx_files_hash` is left out on purpose and built after the first run; see +/// [`crate::db::repo::build_hash_index`]. +/// /// `documents_text` holds a row only when the extractor produced text (an /// audio file with empty tags is done but has none), so `files.content_state` /// is the authority on "content indexed". The uncompressed length is not @@ -203,7 +206,6 @@ CREATE TABLE files ( ); CREATE UNIQUE INDEX idx_files_parent ON files(parent, name); -CREATE INDEX idx_files_hash ON files(hash); CREATE INDEX idx_files_content_pending ON files(id) WHERE content_state = 0; CREATE TABLE failed_files ( diff --git a/crates/quicksearch-core/src/extract/audio.rs b/crates/quicksearch-core/src/extract/audio.rs index bf4fc81..cff2a06 100644 --- a/crates/quicksearch-core/src/extract/audio.rs +++ b/crates/quicksearch-core/src/extract/audio.rs @@ -4,6 +4,7 @@ use std::path::Path; use lofty::{ + config::ParseOptions, file::TaggedFileExt, probe::Probe, tag::{Accessor, ItemKey}, @@ -24,8 +25,14 @@ impl Extractor for AudioExtractor { out: &mut String, _scratch: &mut Scratch, ) -> Result<(), ExtractError> { + // Tags only: the audio properties (duration, bitrate) cost a seek to + // the end and, in an MP3 without a Xing header, a frame-sync search + // back from it, and fail a file whose audio is cut short. Measured + // (`extractprobe`, 18 generated 3-minute MP3s and 40 Ogg sounds): + // `lseek` + `read` calls 2,567 → 790, cached throughput 1.4–2.3x. let tagged = Probe::open(path) .map_err(|e| format!("lofty probe {}: {}", path.display(), e))? + .options(ParseOptions::new().read_properties(false)) .read() .map_err(|e| format!("lofty read {}: {}", path.display(), e))?; @@ -125,6 +132,17 @@ mod tests { assert_eq!(out, "Blue Monday New Order Power Corruption Synthpop"); } + /// A download cut short keeps its tag: the audio after it is never read. + #[test] + fn a_truncated_stream_still_yields_its_tags() { + let path = write_mp3("audio-truncated", &[("TIT2", "Half Written")]); + let bytes = std::fs::read(&path).unwrap(); + // The tag, the first frame's header and a few bytes of its body. + let tag_end = bytes.len() - 4 * 417; + std::fs::write(&path, &bytes[..tag_end + 10]).unwrap(); + assert_eq!(extract(&path).expect("extract"), "Half Written"); + } + #[test] fn an_untagged_file_yields_empty_text() { let path = write_mp3("audio-untagged", &[]); diff --git a/crates/quicksearch-core/src/extract/mod.rs b/crates/quicksearch-core/src/extract/mod.rs index c5e814d..6d727b4 100644 --- a/crates/quicksearch-core/src/extract/mod.rs +++ b/crates/quicksearch-core/src/extract/mod.rs @@ -72,12 +72,10 @@ impl Limits { pub struct Scratch { /// The head bytes a walk worker hashes and sniffs each file from. head: Vec, - /// One container member or one whole file, for a parser that will not - /// take a reader. Grows to the largest member the worker has met and - /// stays there, bounded by [`Limits::inflate`]. + /// One whole file, for a parser that will not take a reader. Grows to + /// the largest file the worker has met and stays there. bytes: Vec, - /// quick-xml's per-event buffer. Separate from `bytes` because a - /// container walk holds both at once. + /// quick-xml's per-event buffer. events: Vec, /// XLSX's shared-string table. Cleared between files; the entries keep /// their capacity, which is most of what a table costs. @@ -110,24 +108,22 @@ impl Scratch { &self.head } - /// The raw-bytes buffer: one container member, one OLE stream, or one - /// whole file for a format whose parser will not take a reader. The - /// caller clears it before filling. + /// The raw-bytes buffer: one whole file for a format whose parser will + /// not take a reader. The caller clears it before filling. pub(crate) fn bytes_mut(&mut self) -> &mut Vec { &mut self.bytes } - /// The member and event buffers together, which a container walk holds - /// at once; two `&mut self` accessors could not lend both. - pub(crate) fn container_bufs(&mut self) -> (&mut Vec, &mut Vec) { - (&mut self.bytes, &mut self.events) + /// quick-xml's event buffer, for a container's XML members. + pub(crate) fn events(&mut self) -> &mut Vec { + &mut self.events } - /// The same pair plus the shared-string table, cleared, which XLSX needs - /// all at once. - pub(crate) fn xlsx_bufs(&mut self) -> (&mut Vec, &mut Vec, &mut Vec) { + /// The event buffer plus the shared-string table, cleared, which XLSX + /// needs at once; two `&mut self` accessors could not lend both. + pub(crate) fn xlsx_bufs(&mut self) -> (&mut Vec, &mut Vec) { self.strings.clear(); - (&mut self.bytes, &mut self.events, &mut self.strings) + (&mut self.events, &mut self.strings) } } diff --git a/crates/quicksearch-core/src/extract/office.rs b/crates/quicksearch-core/src/extract/office.rs index 1b8be08..9ef4dc7 100644 --- a/crates/quicksearch-core/src/extract/office.rs +++ b/crates/quicksearch-core/src/extract/office.rs @@ -218,59 +218,53 @@ fn open_container(path: &Path) -> Result> { Ok(ZipArchive::new(BufReader::new(File::open(path)?))?) } -/// Inflate one whole member into `buf`, the worker's buffer, reused across -/// members and files so members cost no allocation after the first document. -/// Bounded by `limit` ([`super::Limits::inflate`]); the buffer never shrinks, -/// so one hostile document leaves the worker holding up to `limit` for the -/// rest of the pass. -fn member_bytes( - archive: &mut ZipArchive, +/// One member, inflated as the parser reads it, bounded by `limit` +/// ([`super::Limits::inflate`]): inflation stops where parsing does, at the +/// text limit, and nothing holds a whole member. Measured against inflating +/// each member into a per-worker buffer that kept its largest size +/// (`extractprobe`, 1.25 MB `document.xml` members): peak live per worker +/// 2.3 → 0.4 MiB, VmHWM 20.5 → 14.4 MiB, throughput unchanged. +fn member<'a, R: Read + Seek>( + archive: &'a mut ZipArchive, name: &str, limit: usize, - buf: &mut Vec, -) -> Result<(), Box> { - buf.clear(); - archive - .by_name(name)? - .take(limit as u64) - .read_to_end(buf)?; - Ok(()) +) -> Result> { + Ok(BufReader::new(archive.by_name(name)?.take(limit as u64))) } -/// Names of the `.xml` members under `prefix`, in archive order, not -/// `file_names()`, which iterates a hash map and would shuffle the slides of -/// every presentation. -fn xml_members_under( - archive: &mut ZipArchive, - prefix: &str, -) -> Result, Box> { - let mut names = Vec::new(); - for i in 0..archive.len() { - let name = archive.by_index(i)?.name().to_string(); - if name.starts_with(prefix) && name.ends_with(".xml") { - names.push(name); - } - } - Ok(names) +/// Names of the `.xml` members under `prefix`, in number order: shorter +/// first, so `slide2.xml` comes before `slide10.xml`. From the central +/// directory the archive already holds; naming members through `by_index` +/// seeks to and reads each one's local header, about 3 `lseek` and 2 reads a +/// member, twice for a deck. Measured (`extractprobe`, 20 copies of a real +/// 86-member deck plus 4 other documents): `lseek` + `read` calls 22,289 → +/// 4,444, throughput 1,800 → 2,330 files/s. +fn xml_members_under(archive: &ZipArchive, prefix: &str) -> Vec { + let mut names: Vec = archive + .file_names() + .filter(|name| name.starts_with(prefix) && name.ends_with(".xml")) + .map(str::to_string) + .collect(); + names.sort_by(|a, b| a.len().cmp(&b.len()).then_with(|| a.cmp(b))); + names } /// A format whose whole text lives in one member under one spec. fn single_member( path: &Path, - member: &str, + name: &str, spec: &TextSpec, out: &mut String, scratch: &mut Scratch, ) -> Result<(), Box> { let limits = scratch.limits(); let mut archive = open_container(path)?; - let (bytes, events) = scratch.container_bufs(); - member_bytes(&mut archive, member, limits.inflate, bytes)?; - collect_xml_text(bytes.as_slice(), spec, out, limits.text, events) + let xml = member(&mut archive, name, limits.inflate)?; + collect_xml_text(xml, spec, out, limits.text, scratch.events()) } /// Concatenate what `collect` gets out of each `.xml` member under `prefix`, -/// in archive order, stopping once the text reaches [`super::Limits::text`], +/// in number order, stopping once the text reaches [`super::Limits::text`], /// the point past which the caller would discard it anyway. fn collect_members( archive: &mut Archive, @@ -279,7 +273,7 @@ fn collect_members( limit: usize, mut collect: impl FnMut(&mut Archive, &str, &mut String) -> Result<(), Box>, ) -> Result<(), Box> { - for name in xml_members_under(archive, prefix)? { + for name in xml_members_under(archive, prefix) { if out.len() >= limit { break; } @@ -300,15 +294,15 @@ fn collect_members( fn extract_pptx(path: &Path, out: &mut String, scratch: &mut Scratch) -> Result<(), Box> { let limits = scratch.limits(); let mut archive = open_container(path)?; - let (bytes, events) = scratch.container_bufs(); + let events = scratch.events(); collect_members( &mut archive, "ppt/slides/slide", out, limits.text, |archive, name, out| { - member_bytes(archive, name, limits.inflate, bytes)?; - collect_xml_text(bytes.as_slice(), &PPTX, out, limits.text, events)?; + let xml = member(archive, name, limits.inflate)?; + collect_xml_text(xml, &PPTX, out, limits.text, events)?; out.push_str("\n--- New Slide ---\n"); Ok(()) }, @@ -321,9 +315,9 @@ fn extract_pptx(path: &Path, out: &mut String, scratch: &mut Scratch) -> Result< out, limits.text, |archive, name, out| { - member_bytes(archive, name, limits.inflate, bytes)?; + let xml = member(archive, name, limits.inflate)?; out.push_str("\n--- Speaker Notes ---\n"); - collect_xml_text(bytes.as_slice(), &PPTX, out, limits.text, events) + collect_xml_text(xml, &PPTX, out, limits.text, events) }, ) } @@ -338,14 +332,13 @@ fn extract_pptx(path: &Path, out: &mut String, scratch: &mut Scratch) -> Result< fn shared_strings( archive: &mut ZipArchive, limit: usize, - bytes: &mut Vec, events: &mut Vec, strings: &mut Vec, ) { - if member_bytes(archive, "xl/sharedStrings.xml", limit, bytes).is_err() { + let Ok(xml) = member(archive, "xl/sharedStrings.xml", limit) else { return; - } - let mut reader = Reader::from_reader(bytes.as_slice()); + }; + let mut reader = Reader::from_reader(xml); // No `trim_text`; see `collect_xml_text`. events.clear(); let mut in_text = false; @@ -481,18 +474,18 @@ fn collect_sheet( fn extract_xlsx(path: &Path, out: &mut String, scratch: &mut Scratch) -> Result<(), Box> { let limits = scratch.limits(); let mut archive = open_container(path)?; - // All three at once: the sheets are read while the table is live, and + // Both at once: the sheets are read while the table is live, and // separate `&mut scratch` borrows cannot overlap. - let (bytes, events, strings) = scratch.xlsx_bufs(); - shared_strings(&mut archive, limits.inflate, bytes, events, strings); + let (events, strings) = scratch.xlsx_bufs(); + shared_strings(&mut archive, limits.inflate, events, strings); collect_members( &mut archive, "xl/worksheets/sheet", out, limits.text, |archive, name, out| { - member_bytes(archive, name, limits.inflate, bytes)?; - collect_sheet(bytes.as_slice(), strings, out, limits.text, events) + let xml = member(archive, name, limits.inflate)?; + collect_sheet(xml, strings, out, limits.text, events) }, ) } @@ -796,7 +789,7 @@ mod tests { "; #[test] - fn pptx_marks_each_slide_and_keeps_archive_order() { + fn pptx_marks_each_slide() { let p = container( "pptx", "pptx", @@ -811,6 +804,36 @@ mod tests { ); } + /// Slides come out in number order whatever order the archive stores + /// them in, and `slide10` is not read between `slide1` and `slide2`. + #[test] + fn pptx_slides_come_out_in_number_order() { + let slide = |text: &str| { + format!( + "\ + {text}\ + " + ) + }; + let (s10, s2, s1) = (slide("ten"), slide("two"), slide("one")); + let p = container( + "pptx-order", + "pptx", + &[ + ("ppt/slides/slide10.xml", s10.as_str()), + ("ppt/slides/slide2.xml", s2.as_str()), + ("ppt/slides/slide1.xml", s1.as_str()), + ], + ); + let text = extract_document_text(&p, "pptx").unwrap(); + let slides: Vec<&str> = text + .split("\n--- New Slide ---\n") + .map(str::trim) + .filter(|s| !s.is_empty()) + .collect(); + assert_eq!(slides, ["one", "two", "ten"]); + } + /// Speaker notes live in parts of their own that the slide text never /// mentions. #[test] diff --git a/crates/quicksearch-core/src/extract/plaintext.rs b/crates/quicksearch-core/src/extract/plaintext.rs index e8b14d8..d8b3b24 100644 --- a/crates/quicksearch-core/src/extract/plaintext.rs +++ b/crates/quicksearch-core/src/extract/plaintext.rs @@ -56,6 +56,11 @@ pub struct PlaintextExtractor; /// Read `size` bytes from `f`, never more than `cap`. A short read is not an /// error: a file that shrank keeps its prefix. +/// +/// Zero-filled, and still the faster shape: `take(size).read_to_end` into a +/// reserved `Vec` skips the fill but starts at 8 KiB reads and doubles, so +/// over `/usr/include` it made 18,431 `read` calls against 15,419 here and +/// ran 8% slower (`extractprobe`). Measured and rejected. fn read_sized(f: &mut File, size: usize, cap: usize, path: &Path) -> Result, ExtractError> { let size = size.min(cap); let mut buf = vec![0u8; size]; diff --git a/crates/quicksearch-core/src/file_handling/batch.rs b/crates/quicksearch-core/src/file_handling/batch.rs index 2f4ded8..c6adedd 100644 --- a/crates/quicksearch-core/src/file_handling/batch.rs +++ b/crates/quicksearch-core/src/file_handling/batch.rs @@ -10,112 +10,53 @@ use super::*; use crate::config::Config; use crate::db::repo; -/// One writer's compressed sidecars: a batch's blobs end to end in `arena`, -/// with `slots[i]` saying where row `i`'s is, that it has none, or that its -/// compression failed (kept per row rather than failing the batch). +/// Write one batch of prepared records in one transaction through +/// `write_row`, storing any inline text on the row, which both row writers +/// leave pending. `enc` is the run's one encoder (see [`store_extracted`]). /// -/// One per indexing run, reused by every batch and turn the writer takes: the -/// encoder because a zstd context per chunk is wasted CPU (`benches/index.rs`, -/// `zstd_encode`), the arena so the writer allocates nothing per document. -/// Measured against one per writer turn (`indexprobe cold`, 10k docs of -/// 9–20 KiB): VmHWM 60–65 → 44–50 MiB, CPU ~2% lower. -pub struct Bodies { - enc: repo::DocEncoder, - arena: Vec, - slots: Vec>, String>>, -} - -impl Bodies { - pub fn new() -> Result { - Ok(Bodies { - enc: repo::DocEncoder::new()?, - arena: Vec::new(), - slots: Vec::new(), - }) - } - - /// Compress one chunk's bodies before the caller takes the connection, so - /// the lock covers only the SQL. - fn fill<'a>(&mut self, texts: impl Iterator>) { - self.arena.clear(); - self.slots.clear(); - for text in texts { - let slot = match text { - Some(t) if !t.is_empty() => self.enc.encode_into(t, &mut self.arena).map(Some), - _ => Ok(None), - }; - self.slots.push(slot); - } - } - - /// Row `i`'s blob, or why there is none. - fn get(&self, i: usize) -> Result, &str> { - match &self.slots[i] { - Ok(Some(at)) => Ok(Some(&self.arena[at.clone()])), - Ok(None) => Ok(None), - Err(e) => Err(e), - } - } -} - -/// The skeleton behind [`process_batch_updates`] and -/// [`process_batch_inserts`]: compress each chunk's bodies outside the lock, -/// write its rows in one transaction through `write_row`, and store any -/// inline text on the row, which both row writers leave pending. +/// Writing an inline row as done outright would save the claim's `UPDATE`, +/// but it would be a second way out of pending beside +/// [`repo::complete_pending`], for 0.5–1.1 µs a row: about 0.1% of a cold +/// run's CPU, below `memprobe`'s noise. Measured and rejected. fn write_prepared_records( conn_mutex: &Arc>, records: &[OwnedNewFile], stop_flag: &Arc, - config: &Config, - bodies: &mut Bodies, + enc: &mut repo::DocEncoder, write_row: impl Fn(&rusqlite::Transaction<'_>, &OwnedNewFile) -> Result, String>, ) -> Result<(), String> { - // `.max(1)`: `chunks(0)` panics on the indexing thread, so a hand-edited - // `batch_size = 0` would wedge indexing while the UI reads "Running". - let chunk_size = config.processing.batch_size.max(1); - for batch in records.chunks(chunk_size) { + let conn = crate::lock_ok(conn_mutex); + let tx = conn + .unchecked_transaction() + .map_err(|e| format!("Failed to begin transaction: {}", e))?; + + for rec in records { if stop_flag.load(Ordering::Relaxed) { return Ok(()); } - - // Outside the lock; see `Bodies::fill`. - bodies.fill(batch.iter().map(|r| r.inline_text.as_deref())); - let conn = crate::lock_ok(conn_mutex); - let tx = conn - .unchecked_transaction() - .map_err(|e| format!("Failed to begin transaction: {}", e))?; - - for (i, rec) in batch.iter().enumerate() { - if stop_flag.load(Ordering::Relaxed) { - return Ok(()); - } - let id = write_row(&tx, rec)?; - if let (Some(id), Some(text)) = (id, rec.inline_text.as_deref()) { - match bodies.get(i) { - Ok(zstd) => { - repo::store_pending_text(&tx, id, text, zstd)?; - } - Err(e) => crate::log_warn!("compress text for {}: {}", rec.path(), e), + let id = write_row(&tx, rec)?; + if let (Some(id), Some(text)) = (id, rec.inline_text.as_deref()) { + match enc.body(text) { + Ok(zstd) => { + repo::store_pending_text(&tx, id, text, zstd)?; } + Err(e) => crate::log_warn!("compress text for {}: {}", rec.path(), e), } } - - tx.commit() - .map_err(|e| format!("Failed to commit transaction: {}", e))?; } - Ok(()) + tx.commit() + .map_err(|e| format!("Failed to commit transaction: {}", e)) } -/// Write already-prepared records for files whose content changed. No -/// filesystem I/O; records carrying inline text are stored complete here, -/// the rest stay pending. +/// Write one batch of already-prepared records for files whose content +/// changed. No filesystem I/O; records carrying inline text are stored +/// complete here, the rest stay pending. pub fn process_batch_updates( conn_mutex: &Arc>, files_to_update: &[OwnedNewFile], stop_flag: &Arc, - config: &Config, - bodies: &mut Bodies, + enc: &mut repo::DocEncoder, ) -> Result<(), String> { if files_to_update.is_empty() { return Ok(()); @@ -124,45 +65,37 @@ pub fn process_batch_updates( // The row leaves this closure pending whenever it carries inline text: // `update_file_basic` cleared a DONE row's content and reset its state // in this same transaction, and the insert fallback created it pending. - write_prepared_records( - conn_mutex, - files_to_update, - stop_flag, - config, - bodies, - |tx, rec| { - let updated = repo::update_file_basic(tx, &rec.as_new_file()).map_err(|e| { - format!( - "Failed to update file record + clear stale content for {}: {}", - rec.path(), - e - ) - })?; + write_prepared_records(conn_mutex, files_to_update, stop_flag, enc, |tx, rec| { + let updated = repo::update_file_basic(tx, &rec.as_new_file()).map_err(|e| { + format!( + "Failed to update file record + clear stale content for {}: {}", + rec.path(), + e + ) + })?; - // No row matched: dropping the update would leave the mtime stale - // and the file re-hashed on every run forever. - match updated { - None => { - crate::log_warn!( - "no indexed row matched {} during update; inserting instead", - rec.path() - ); - repo::insert_file(tx, &rec.as_new_file()) - .map_err(|e| format!("Failed to insert file record: {}", e)) - } - some => Ok(some), + // No row matched: dropping the update would leave the mtime stale + // and the file re-hashed on every run forever. + match updated { + None => { + crate::log_warn!( + "no indexed row matched {} during update; inserting instead", + rec.path() + ); + repo::insert_file(tx, &rec.as_new_file()) + .map_err(|e| format!("Failed to insert file record: {}", e)) } - }, - ) + some => Ok(some), + } + }) } -/// Write already-prepared records for newly discovered files. +/// Write one batch of already-prepared records for newly discovered files. pub fn process_batch_inserts( conn_mutex: &Arc>, files_to_insert: &[OwnedNewFile], stop_flag: &Arc, - config: &Config, - bodies: &mut Bodies, + enc: &mut repo::DocEncoder, ) -> Result<(), String> { if files_to_insert.is_empty() { return Ok(()); @@ -170,17 +103,10 @@ pub fn process_batch_inserts( // `insert_file` returned `Some` only by creating the row, pending when it // needs content, so it cannot carry content from anywhere. - write_prepared_records( - conn_mutex, - files_to_insert, - stop_flag, - config, - bodies, - |tx, rec| { - repo::insert_file(tx, &rec.as_new_file()) - .map_err(|e| format!("Failed to insert file record: {}", e)) - }, - ) + write_prepared_records(conn_mutex, files_to_insert, stop_flag, enc, |tx, rec| { + repo::insert_file(tx, &rec.as_new_file()) + .map_err(|e| format!("Failed to insert file record: {}", e)) + }) } /// Delete the rows a completed run found no file behind; returns how many @@ -200,6 +126,10 @@ pub fn process_batch_inserts( /// Extraction may still be running (its writes serialize on this connection /// mutex), so between chunks a doomed row can flip PENDING→DONE; resolved any /// earlier, its posting would outlive the row. +/// +/// Not worth folding the lookup into the delete (`DELETE … RETURNING` per +/// path): the lookups are 35–52 ms of 1.4–1.5 s over the 40k paths above, +/// where the commit, FTS5 writing its tombstones, is 1.0–1.2 s. pub fn cleanup_stale_index_entries( conn_mutex: &Arc>, stale_paths: &[String], @@ -357,11 +287,6 @@ pub fn count_extract_scope( }) } -/// Rows per compression chunk and per transaction inside [`store_extracted`]: -/// half of what a writer turn may hand in (`pipeline::READY_TOPUP` is 64), so -/// a full turn commits twice and each hold of the connection stays short. -const STORE_CHUNK: usize = 32; - /// What one [`store_extracted`] call did with the rows it was handed. #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] pub struct Stored { @@ -373,57 +298,56 @@ pub struct Stored { } /// Write already-extracted rows, all that runs with the connection held, -/// until `deadline`. FTS5 tokenization happens here: the writer thread's -/// dominant cost. A turn overruns the deadline by at most one +/// until `deadline`, in one transaction. FTS5 tokenization happens here: the +/// writer thread's dominant cost. A turn overruns the deadline by at most one /// document; at least one row is always consumed unless the run is stopped, /// so a caller looping on this cannot spin. A row whose write fails is /// logged and consumed; its `content_state` stays pending for the next run. +/// +/// `enc` is the run's one encoder, compressing each body as its row is +/// written: a zstd context per call is wasted CPU (`benches/index.rs`, +/// `zstd_encode`), and nothing else takes the connection during a run but +/// `stop_indexing`'s checkpoint, so compressing outside the lock bought +/// nothing. Measured against compressing each 32-row chunk into an arena +/// first and committing per chunk (`memprobe cold`): bytes written 855 → 825 +/// MiB on the cargo registry (41k files) and 2.88 → 2.81 GB on `/usr` (373k), +/// peak RSS 180–200 → 174–176 MiB, CPU within noise. pub fn store_extracted( conn_mutex: &Arc>, rows: &[crate::content::ExtractedRow], stop_flag: &Arc, deadline: std::time::Instant, - bodies: &mut Bodies, + enc: &mut repo::DocEncoder, ) -> Result { let mut done = Stored::default(); - for chunk in rows.chunks(STORE_CHUNK) { - if stop_flag.load(Ordering::Relaxed) { - break; - } - // Outside the lock; see `Bodies::fill`. - bodies.fill( - chunk - .iter() - .map(|r| crate::file_handling::outcome_body(&r.outcome)), - ); - let conn = crate::lock_ok(conn_mutex); - let tx = conn - .unchecked_transaction() - .map_err(|e| format!("Failed to begin transaction: {}", e))?; - let mut cut = false; - for (i, row) in chunk.iter().enumerate() { - // Counted before anything can skip it: a failed row still leaves. - done.consumed += 1; - match bodies.get(i) { - Err(e) => crate::log_warn!("compress text for {}: {}", row.path(), e), - Ok(zstd) => match repo::complete_pending(&tx, row.file_id, &row.outcome, zstd) { + if stop_flag.load(Ordering::Relaxed) { + return Ok(done); + } + let conn = crate::lock_ok(conn_mutex); + let tx = conn + .unchecked_transaction() + .map_err(|e| format!("Failed to begin transaction: {}", e))?; + for row in rows { + // Counted before anything can skip it: a failed row still leaves. + done.consumed += 1; + let body = crate::file_handling::outcome_body(&row.outcome).map(|text| enc.body(text)); + match body.transpose() { + Err(e) => crate::log_warn!("compress text for {}: {}", row.path(), e), + Ok(zstd) => { + match repo::complete_pending(&tx, row.file_id, &row.outcome, zstd.flatten()) { Ok(true) => done.written += 1, // No longer pending: stored or deleted since it was paged. Ok(false) => {} Err(e) => crate::log_warn!("content indexing for {}: {}", row.path(), e), - }, - } - if stop_flag.load(Ordering::Relaxed) || std::time::Instant::now() >= deadline { - cut = true; - break; + } } } - tx.commit() - .map_err(|e| format!("Failed to commit transaction: {}", e))?; - if cut { + if stop_flag.load(Ordering::Relaxed) || std::time::Instant::now() >= deadline { break; } } + tx.commit() + .map_err(|e| format!("Failed to commit transaction: {}", e))?; Ok(done) } diff --git a/crates/quicksearch-core/src/file_handling/count_and_extract_tests.rs b/crates/quicksearch-core/src/file_handling/count_and_extract_tests.rs index 1ec4788..56dec31 100644 --- a/crates/quicksearch-core/src/file_handling/count_and_extract_tests.rs +++ b/crates/quicksearch-core/src/file_handling/count_and_extract_tests.rs @@ -235,8 +235,7 @@ fn extract_scope_counts_only_files_an_extractor_claims() { &conn_mutex, &records, &stop, - &config, - &mut Bodies::new().unwrap(), + &mut crate::db::repo::DocEncoder::new().unwrap(), ) .unwrap(); diff --git a/crates/quicksearch-core/src/file_handling/mod.rs b/crates/quicksearch-core/src/file_handling/mod.rs index de7fcd5..251a5a2 100644 --- a/crates/quicksearch-core/src/file_handling/mod.rs +++ b/crates/quicksearch-core/src/file_handling/mod.rs @@ -16,7 +16,7 @@ pub(crate) use batch::extract_scope_prepare; pub(crate) use batch::max_text_file_size; pub use batch::{ cleanup_stale_index_entries, count_extract_scope, mark_oversize_pending_na, - process_batch_inserts, process_batch_updates, store_extracted, Bodies, ExtractCursor, + process_batch_inserts, process_batch_updates, store_extracted, ExtractCursor, ExtractScope, RootRange, Stored, }; pub use counting::count_tree_entries_fast; @@ -29,7 +29,7 @@ pub(crate) use paths::{ }; pub(crate) use records::mtime_secs; pub use records::{ - classify_by_mtime, classify_for_indexing, content_extractable, decide_content, + classify_by_mtime, content_extractable, decide_content, extract_and_store, fts_begin_bulk_write, fts_begin_tombstone_burst, fts_end_tombstone_burst, fts_finalize_after_text_indexing, get_file_hash, hash_failure_counts, outcome_body, prepare_file_record, reset_run_warnings, ContentOutcome, DirRows, FileIndexAction, diff --git a/crates/quicksearch-core/src/file_handling/records.rs b/crates/quicksearch-core/src/file_handling/records.rs index 4874078..621f236 100644 --- a/crates/quicksearch-core/src/file_handling/records.rs +++ b/crates/quicksearch-core/src/file_handling/records.rs @@ -26,14 +26,10 @@ pub enum FileIndexAction { Insert, } -/// Decide what Phase 1 should do with a file. A file whose parent is not this -/// directory (a resolved symlink target) must not be classified here: it -/// would read as `Insert`, and `INSERT OR IGNORE` would silently not update -/// it. Use [`classify_by_mtime`] for those. -pub fn classify_for_indexing(name: &str, mtime: u64, rows: &DirRows) -> FileIndexAction { - classify_by_mtime(rows.get(name).copied(), mtime) -} - +/// Decide what Phase 1 should do with a file from its stored mtime, `None` +/// when it has no row. For a resolved symlink target that is the row under +/// the target's own path: its link's directory rows would read it as +/// `Insert`, and `INSERT OR IGNORE` would silently not update it. pub fn classify_by_mtime(stored: Option, mtime: u64) -> FileIndexAction { match stored { Some(known) if known == mtime => FileIndexAction::Skip, diff --git a/crates/quicksearch-core/src/file_handling/tests.rs b/crates/quicksearch-core/src/file_handling/tests.rs index baad32c..5f2f803 100644 --- a/crates/quicksearch-core/src/file_handling/tests.rs +++ b/crates/quicksearch-core/src/file_handling/tests.rs @@ -170,31 +170,8 @@ fn filtered_walk_hidden_root_still_walked() { } #[test] -fn classify_uses_mtime_against_the_existing_index() { - let mut rows = DirRows::new(); - rows.insert("known.txt".to_string(), 100); - - assert_eq!( - classify_for_indexing("new.txt", 100, &rows), - FileIndexAction::Insert, - "a name absent from the directory's rows is new" - ); - assert_eq!( - classify_for_indexing("known.txt", 100, &rows), - FileIndexAction::Skip, - "same mtime means nothing to do" - ); - assert_eq!( - classify_for_indexing("known.txt", 101, &rows), - FileIndexAction::Update, - "a changed mtime means re-read" - ); -} - -#[test] -fn classify_by_mtime_matches_the_name_keyed_path() { - // Resolved symlink targets take this route: their row is found by - // exact path. +fn classify_uses_mtime_against_the_stored_row() { + // No row: new. Same mtime: nothing to do. Changed: re-read. assert_eq!(classify_by_mtime(None, 100), FileIndexAction::Insert); assert_eq!(classify_by_mtime(Some(100), 100), FileIndexAction::Skip); assert_eq!(classify_by_mtime(Some(99), 100), FileIndexAction::Update); diff --git a/crates/quicksearch-core/src/incremental.rs b/crates/quicksearch-core/src/incremental.rs index 12f71b7..8116159 100644 --- a/crates/quicksearch-core/src/incremental.rs +++ b/crates/quicksearch-core/src/incremental.rs @@ -6,7 +6,7 @@ use std::path::Path; use std::sync::atomic::{AtomicBool, Ordering}; use std::time::Instant; -use rusqlite::Connection; +use rusqlite::Transaction; use crate::config::{Config, IgnoreSet}; use crate::db::repo; @@ -40,19 +40,19 @@ impl Budget<'_> { #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Applied { Done, - /// Budget spent. What was written is committed; the caller should re-queue - /// the same event with `resume_from` set to `done`. + /// Budget spent. What was written stays written; the caller should + /// re-queue the same event with `resume_from` set to `done`. Unfinished { done: usize, }, } -/// Apply one filesystem event to the index. Missing files are treated as -/// no-ops (the pending Remove event resolves them); unchanged mtimes -/// short-circuit without touching the DB. See [`Budget`] for the one event -/// that is not small. +/// Apply one filesystem event to the index, inside the caller's transaction, +/// which may hold many events. Missing files are treated as no-ops (the +/// pending Remove event resolves them); unchanged mtimes short-circuit without +/// touching the DB. See [`Budget`] for the one event that is not small. pub fn apply_fs_event( - conn: &mut Connection, + tx: &Transaction<'_>, event: &FsEvent, config: &Config, ignore: &IgnoreSet, @@ -61,18 +61,18 @@ pub fn apply_fs_event( ) -> Result { match event { FsEvent::Create(p) | FsEvent::Modify(p) => { - upsert_path(conn, p, config, ignore, registry, budget) + upsert_path(tx, p, config, ignore, registry, budget) } - FsEvent::Remove(p) => remove_path(conn, p).map(|()| Applied::Done), + FsEvent::Remove(p) => remove_paths(tx, std::slice::from_ref(p)).map(|()| Applied::Done), FsEvent::Rename { from, to } => { - remove_path(conn, from)?; - upsert_path(conn, to, config, ignore, registry, budget) + remove_paths(tx, std::slice::from_ref(from))?; + upsert_path(tx, to, config, ignore, registry, budget) } } } fn upsert_path( - conn: &mut Connection, + tx: &Transaction<'_>, path: &Path, config: &Config, ignore: &IgnoreSet, @@ -127,17 +127,37 @@ fn upsert_path( if budget.spent() { return Ok(Applied::Unfinished { done }); } - upsert_file(conn, entry.path(), config, registry)?; + upsert_file(tx, entry.path(), config, registry)?; done += 1; } Ok(Applied::Done) } else { - upsert_file(conn, path, config, registry).map(|()| Applied::Done) + upsert_file(tx, path, config, registry).map(|()| Applied::Done) + } +} + +/// Run `write` all or nothing inside the caller's transaction: a file whose +/// writes fail part-way must not keep a row that claims text its posting +/// never got. +fn atomically( + tx: &Transaction<'_>, + write: impl FnOnce() -> Result<(), String>, +) -> Result<(), String> { + tx.execute_batch("SAVEPOINT one_path") + .map_err(|e| format!("begin incremental savepoint: {}", e))?; + match write() { + Ok(()) => tx + .execute_batch("RELEASE one_path") + .map_err(|e| format!("release incremental savepoint: {}", e)), + Err(e) => { + let _ = tx.execute_batch("ROLLBACK TO one_path; RELEASE one_path"); + Err(e) + } } } fn upsert_file( - conn: &mut Connection, + tx: &Transaction<'_>, path: &Path, config: &Config, registry: &Registry, @@ -153,16 +173,12 @@ fn upsert_file( return Ok(()); }; - let tx = conn - .transaction() - .map_err(|e| format!("begin incremental tx: {}", e))?; - // Settled before the file is opened, as the full walk settles it: an // unchanged file (a re-sent event, a `chmod`, the live view re-checking a // row it shows) costs one lookup, never a read and a hash. Measured - // (`benches/index.rs`, `unchanged_event`, 500 small text files): 3.3 ms - // hashing and sniffing each against 1.4 ms. - let stored = repo::mtime_for_path(&tx, &db_path)?; + // (`benches/index.rs`, `directory_event::unchanged`, 500 small text + // files): 3.3 ms hashing and sniffing each against 1.4 ms. + let stored = repo::mtime_for_path(tx, &db_path)?; if stored.is_some() && stored == mtime_secs(&meta) { return Ok(()); } @@ -171,67 +187,54 @@ fn upsert_file( let Some(rec) = prepare_file_record(&db_path, &meta, config, registry, &mut scratch) else { return Ok(()); }; - let written = match stored { - Some(_) => repo::update_file_basic(&tx, &rec.as_new_file())?, - None => repo::insert_file(&tx, &rec.as_new_file())?, - }; - // `None`: lost a race with another writer; the row that won is current - // enough. - let Some(file_id) = written else { - return Ok(()); - }; + atomically(tx, || { + let written = match stored { + Some(_) => repo::update_file_basic(tx, &rec.as_new_file())?, + None => repo::insert_file(tx, &rec.as_new_file())?, + }; + // `None`: lost a race with another writer; the row that won is + // current enough. + let Some(file_id) = written else { + return Ok(()); + }; - // A row that needs no content is already NA: both writers above set its - // state from `rec`. That gate is also the only size gate on this path: - // `decide_content` has none, and falling through would read a - // multi-gigabyte `.txt` whole into memory. - if rec.needs_content { - match rec.inline_text.as_deref() { - // `prepare_file_record` already read the whole file. - Some(text) => { - let zstd = repo::encode_one(text)?; - repo::store_pending_text(&tx, file_id, text, zstd.as_deref())?; + // A row that needs no content is already NA: both writers above set + // its state from `rec`. That gate is also the only size gate on this + // path: `decide_content` has none, and falling through would read a + // multi-gigabyte `.txt` whole into memory. + if rec.needs_content { + match rec.inline_text.as_deref() { + // `prepare_file_record` already read the whole file. + Some(text) => { + let zstd = repo::encode_one(text)?; + repo::store_pending_text(tx, file_id, text, zstd.as_deref())?; + } + None => extract_and_store(tx, file_id, rec.path(), rec.mime, registry, config)?, } - None => extract_and_store(&tx, file_id, rec.path(), rec.mime, registry, config)?, } - } - - tx.commit() - .map_err(|e| format!("commit incremental tx: {}", e)) + Ok(()) + }) } -fn remove_path(conn: &mut Connection, path: &Path) -> Result<(), String> { - remove_paths(conn, std::slice::from_ref(&path.to_path_buf()), 1) -} - -/// Delete `paths` and everything indexed beneath them, in transactions of at -/// most `chunk` paths. Pays off only when the caller has reduced the set to -/// its *roots*: then `rm -rf dir/` is a fixed handful of range-driven -/// statements ([`repo::delete_subtree`]) rather than five per file. -pub fn remove_paths( - conn: &mut Connection, - paths: &[std::path::PathBuf], - chunk: usize, -) -> Result<(), String> { - for batch in paths.chunks(chunk.max(1)) { - let tx = conn - .transaction() - .map_err(|e| format!("begin incremental tx: {}", e))?; - for path in batch { - // A path the index cannot spell was never indexed, so there is - // nothing to delete; its lossy spelling names another file (see - // `path_to_db_string`), whose row and subtree this would delete. - if path.to_str().is_none() { - continue; - } - // The stored key is canonicalized, but the file is already gone, - // so `canonicalize` cannot run; `db_key_for_missing_path` approximates it. - let path_str = db_key_for_missing_path(path); - repo::delete_file_by_path(&tx, &path_str)?; - repo::delete_subtree(&tx, &RootRange::for_root(&path_str))?; +/// Delete `paths` and everything indexed beneath them, inside the caller's +/// transaction. Pays off only when the caller has reduced the set to its +/// *roots*: then `rm -rf dir/` is a fixed handful of range-driven statements +/// ([`repo::delete_subtree`]) rather than five per file. +pub fn remove_paths(tx: &Transaction<'_>, paths: &[std::path::PathBuf]) -> Result<(), String> { + for path in paths { + // A path the index cannot spell was never indexed, so there is + // nothing to delete; its lossy spelling names another file (see + // `path_to_db_string`), whose row and subtree this would delete. + if path.to_str().is_none() { + continue; } - tx.commit() - .map_err(|e| format!("commit incremental tx: {}", e))?; + // The stored key is canonicalized, but the file is already gone, so + // `canonicalize` cannot run; `db_key_for_missing_path` approximates it. + let path_str = db_key_for_missing_path(path); + atomically(tx, || { + repo::delete_file_by_path(tx, &path_str)?; + repo::delete_subtree(tx, &RootRange::for_root(&path_str)).map(|_| ()) + })?; } Ok(()) } @@ -240,7 +243,7 @@ pub fn remove_paths( mod tests { use super::*; use crate::db::open_or_recreate; - use rusqlite::OptionalExtension; + use rusqlite::{Connection, OptionalExtension}; use std::time::Duration; struct Fixture { @@ -281,8 +284,9 @@ mod tests { } fn apply_resuming(&mut self, event: &FsEvent, budget: Duration, from: usize) -> Applied { - apply_fs_event( - &mut self.conn, + let tx = self.conn.transaction().unwrap(); + let applied = apply_fs_event( + &tx, event, &self.config, &self.ignore, @@ -293,7 +297,9 @@ mod tests { resume_from: from, }, ) - .unwrap() + .unwrap(); + tx.commit().unwrap(); + applied } fn write(&self, name: &str, content: &str) -> std::path::PathBuf { @@ -429,7 +435,9 @@ mod tests { ]; // Collapsed to its root, as the coordinator's `collapse_pending_removals` does. let roots = vec![reported[0].clone()]; - remove_paths(&mut f.conn, &roots, 200).unwrap(); + let tx = f.conn.transaction().unwrap(); + remove_paths(&tx, &roots).unwrap(); + tx.commit().unwrap(); assert_eq!(f.counts(), (1, 1, 1), "only tree2 survives"); let survivor = f.canonical(&f.dir.join("tree2").join("keep.txt")); assert!(f.row(&survivor).is_some()); @@ -690,8 +698,9 @@ mod tests { } std::fs::write(bad.join("inside.txt"), "body").unwrap(); + let tx = f.conn.transaction().unwrap(); let outcome = apply_fs_event( - &mut f.conn, + &tx, &FsEvent::Create(bad), &f.config, &f.ignore, @@ -707,6 +716,7 @@ mod tests { Ok(Applied::Done), "an unindexable subtree is not a failure" ); + tx.commit().unwrap(); assert_eq!(f.counts().0, 0, "and nothing under it is indexed"); } diff --git a/crates/quicksearch-core/src/indexing/pipeline.rs b/crates/quicksearch-core/src/indexing/pipeline.rs index e0b9cc3..6dbea8a 100644 --- a/crates/quicksearch-core/src/indexing/pipeline.rs +++ b/crates/quicksearch-core/src/indexing/pipeline.rs @@ -16,8 +16,8 @@ use crate::extract::Registry; use crate::file_handling::{ cleanup_stale_index_entries, count_tree_entries_fast, fts_begin_bulk_write, fts_finalize_after_text_indexing, mark_oversize_pending_na, normalize_root_string, - process_batch_inserts, process_batch_updates, store_extracted, Bodies, ExtractCursor, - ExtractScope, FileIndexAction, OwnedNewFile, RootRange, + process_batch_inserts, process_batch_updates, store_extracted, ExtractCursor, ExtractScope, + FileIndexAction, OwnedNewFile, RootRange, }; use crate::walk::{thread_count_for, walk_indexable_files, ParallelWalk, TryNext, WalkEvent}; @@ -153,8 +153,7 @@ type WriteBatch = fn( &Arc>, &[OwnedNewFile], &Arc, - &Config, - &mut Bodies, + &mut crate::db::repo::DocEncoder, ) -> Result<(), String>; /// Write `rows` through `write`, then clear them; on an error they stay. @@ -163,13 +162,7 @@ fn flush( rows: &mut Vec, write: WriteBatch, ) -> Result<(), String> { - write( - &cx.conn_mutex, - rows, - cx.stop_flag, - cx.config, - &mut cx.bodies, - )?; + write(&cx.conn_mutex, rows, cx.stop_flag, &mut cx.encoder)?; rows.clear(); Ok(()) } @@ -525,7 +518,7 @@ impl RootPipeline { ready, cx.stop_flag, deadline, - &mut cx.bodies, + &mut cx.encoder, )?; if stored.consumed > 0 { // The last row *written*, not the last fetched. The whole path, @@ -574,8 +567,8 @@ pub(super) struct RunCx<'a> { /// Writer time one root's turn may take before the round moves on /// ([`crate::config::ProcessingConfig::writer_turn_slice_ms`]). pub(super) slice: Duration, - /// The run's one set of compression buffers, shared by every write. - pub(super) bodies: Bodies, + /// The run's one compression context, shared by every write. + pub(super) encoder: crate::db::repo::DocEncoder, /// Rows with no file behind them, per-directory plus the vanished sweep. pub(super) stale_candidates: Vec, /// Paths reached via symlink; their rows may live outside every root. @@ -600,7 +593,7 @@ impl<'a> RunCx<'a> { registry: Arc::new(Registry::default_set()), batch: config.processing.batch_size.max(1), slice: Duration::from_millis(config.processing.writer_turn_slice_ms), - bodies: Bodies::new()?, + encoder: crate::db::repo::DocEncoder::new()?, stale_candidates: Vec::new(), aliased_paths: HashSet::new(), stale_cleanup_ok: true, diff --git a/crates/quicksearch-core/src/live.rs b/crates/quicksearch-core/src/live.rs index 0ab791b..49c0bc6 100644 --- a/crates/quicksearch-core/src/live.rs +++ b/crates/quicksearch-core/src/live.rs @@ -474,12 +474,10 @@ impl Loop { return WindowUpdate::Unchanged; }; let cut = match tier { + // Unlike in the passes, the body may have stopped matching here. ContentTier::Exact => { - // Unlike in the passes, the body may have stopped matching - // here; an unmarked window says so. - let folded = text.to_ascii_lowercase(); - crate::search::cascade::text_snippet(&query.pattern, text, &folded) - .filter(|snip| !snip.ranges.is_empty()) + crate::search::cascade::body_match(&query.pattern, text, &mut String::new()) + .map(|m| m.snippet) } ContentTier::Fuzzy => match &self.fuzzy { Some(bitap) => crate::search::cascade::fuzzy_snippet(bitap, text).map(|(_, s)| s), diff --git a/crates/quicksearch-core/src/query/pattern.rs b/crates/quicksearch-core/src/query/pattern.rs index 3f23979..5e70851 100644 --- a/crates/quicksearch-core/src/query/pattern.rs +++ b/crates/quicksearch-core/src/query/pattern.rs @@ -12,7 +12,6 @@ use regex::{Regex, RegexBuilder}; use super::translator::TranslateError; use crate::search::prefilter::Required; -use crate::snippet; /// Keeps a hostile pattern (`a{1000000}{1000}`) from ballooning the program. const REGEX_SIZE_LIMIT: usize = 4 << 20; @@ -79,6 +78,21 @@ impl WildcardPattern { &self.anchored_cs } } + + /// Non-overlapping occurrences in `text`, stopping at 1000 where the + /// cascade's `count_frac` saturates, and the first of them, in one sweep. + pub fn count_and_first( + &self, + text: &str, + case_insensitive: bool, + ) -> (usize, Option>) { + let mut found = self + .search(case_insensitive) + .find_iter(text) + .take(COUNT_CAP); + let first = found.next().map(|m| m.range()); + (first.as_ref().map_or(0, |_| 1 + found.count()), first) + } } impl TermPattern { @@ -246,38 +260,6 @@ impl TermPattern { TermPattern::Wildcard(w) => w.search(case_insensitive).find(text).map(|m| m.range()), } } - - /// [`TermPattern::find_first`], case-insensitive, on a pre-folded haystack. - pub fn find_first_folded(&self, folded: &str) -> Option> { - match self { - TermPattern::Empty => None, - TermPattern::Literal(l) => { - let pos = folded.find(&l.folded)?; - Some(pos..pos + l.text.len()) - } - TermPattern::Wildcard(w) => w.search_ci.find(folded).map(|m| m.range()), - } - } - - /// Case-insensitive [`TermPattern::count`] against an already-folded haystack. - pub fn count_folded(&self, folded: &str) -> usize { - match self { - TermPattern::Empty => 0, - TermPattern::Literal(l) => snippet::count_occurrences(folded, &l.folded), - TermPattern::Wildcard(w) => w.search_ci.find_iter(folded).take(COUNT_CAP).count(), - } - } - - /// Case-sensitive, non-overlapping occurrence count; the case-insensitive - /// one is [`TermPattern::count_folded`]. Wildcards stop at 1000, where the - /// cascade's `count_frac` saturates anyway. - pub fn count(&self, text: &str) -> usize { - match self { - TermPattern::Empty => 0, - TermPattern::Literal(l) => snippet::count_occurrences(text, &l.text), - TermPattern::Wildcard(w) => w.search_cs.find_iter(text).take(COUNT_CAP).count(), - } - } } /// A compiled `regex:` query. Case-insensitive by default (override with @@ -369,9 +351,13 @@ impl RegexQuery { self.re.find(text).map(|m| m.range()) } - /// Non-overlapping occurrence count, capped at 1000. - pub fn count(&self, text: &str) -> usize { - self.re.find_iter(text).take(COUNT_CAP).count() + /// Non-overlapping occurrences, capped at 1000, and the first of them, + /// in one sweep. Against a count and then a second search for the first + /// (`searchtime`, cargo registry): within noise to 3% faster. + pub fn count_and_first(&self, text: &str) -> (usize, Option>) { + let mut found = self.re.find_iter(text).take(COUNT_CAP); + let first = found.next().map(|m| m.range()); + (first.as_ref().map_or(0, |_| 1 + found.count()), first) } } @@ -425,7 +411,6 @@ mod tests { assert!(p.is_empty(), "{:?}", parts); assert!(!p.whole_match("anything", true)); assert!(p.find_first("anything", true).is_none()); - assert_eq!(p.count("anything"), 0); } } @@ -512,10 +497,14 @@ mod tests { #[test] fn count_is_nonoverlapping_and_capped() { - let p = TermPattern::build(&[part("a*b", true)]).unwrap(); - assert_eq!(p.count("ab ab ab"), 3); + let TermPattern::Wildcard(w) = TermPattern::build(&[part("a*b", true)]).unwrap() else { + panic!("a star makes a wildcard"); + }; + assert_eq!(w.count_and_first("xx ab ab ab", false), (3, Some(3..5))); let many = "ab ".repeat(2000); - assert_eq!(p.count(&many), 1000); + assert_eq!(w.count_and_first(&many, false).0, 1000); + assert_eq!(w.count_and_first("AB aB", false), (0, None), "exact case"); + assert_eq!(w.count_and_first("AB aB", true), (2, Some(0..2))); } #[test] @@ -527,8 +516,6 @@ mod tests { assert_eq!(p.find_first("my Report.pdf", false), Some(3..9)); assert_eq!(p.find_first("my report.pdf", true), Some(3..9)); assert_eq!(p.find_first("my report.pdf", false), None); - assert_eq!(p.count("report Report"), 1); - assert_eq!(p.count_folded("report report"), 2); } #[test] @@ -585,7 +572,8 @@ mod tests { let r = RegexQuery::new("b[aeiou]d").unwrap(); let hay = "bad bed bodkin"; assert_eq!(r.find_first(hay), Some(0..3)); - assert_eq!(r.count(hay), 3); + assert_eq!(r.count_and_first(hay), (3, Some(0..3))); + assert_eq!(r.count_and_first("nothing here"), (0, None)); } #[test] diff --git a/crates/quicksearch-core/src/search/cascade.rs b/crates/quicksearch-core/src/search/cascade.rs index de71207..1775255 100644 --- a/crates/quicksearch-core/src/search/cascade.rs +++ b/crates/quicksearch-core/src/search/cascade.rs @@ -48,39 +48,72 @@ const CANCEL_CHECK_ROWS: usize = 256; /// Snippet window budget: the GUI trims to column width; mouseover shows the rest. const SNIPPET_WINDOW_BYTES: usize = 600; -const SNIPPET_OPTS: snippet::Options = snippet::Options { - approx_bytes: SNIPPET_WINDOW_BYTES, -}; - -/// The Content Match snippet for one document body, cut exactly as the -/// full-text passes cut it; shared with [`crate::live`] so a re-cut matches. -/// `folded` must be `text` ASCII-lowercased; the fold preserves byte length, -/// so offsets found in it slice `text`. -pub fn text_snippet( - pattern: &crate::query::pattern::TermPattern, - text: &str, - folded: &str, -) -> Option { - text_snippet_counted(pattern, text, folded) - .map(|(snip, _)| snip) - .or_else(|| { - pattern.find_first_folded(folded).map(|r| { - // A greedy pattern can match megabytes; clamp before the window. - let r = clamp_match_range(text, r, SNIPPET_WINDOW_BYTES); - snippet::window_around(text, (r.start, r.end), &SNIPPET_OPTS) - }) - }) +/// How one document body matched the term: stage 5 when it occurs exactly +/// as typed, 6 when only in another case; `count` is what the stage ranks +/// on, `snippet` the window around the first occurrence. +#[derive(Debug, Clone)] +pub struct BodyMatch { + pub stage: u8, + pub count: usize, + pub snippet: snippet::Snippet, } -/// [`text_snippet`] for a literal pattern, plus the case-insensitive -/// occurrence count found on the way. `None` when the pattern is not literal. -pub fn text_snippet_counted( +/// The term's match in one document body, verified and cut exactly as the +/// full-text pass does; `None` when it does not occur. Shared with +/// [`crate::live`], so a re-cut matches. `lower` is reused scratch for the +/// folded body a literal is searched in. +/// +/// One sweep per body where there were three: a literal counts both cases in +/// its one folded scan, and a wildcard is never folded, its exact-case regex +/// giving the count and the first match together. Measured against folding +/// every body, then a folded scan, a case-sensitive recount and, for a +/// wildcard, a third scan for the window (`searchtime`, cargo registry, 41k +/// files): `Hash*Map` 103.5–104.2 → 97.4–98.0 ms, `Serial*ize` 56–59 → 53.5; +/// literals within ±2% (`serialize` 38.6–39.2 → 38.7–39.2). +pub fn body_match( pattern: &crate::query::pattern::TermPattern, text: &str, - folded: &str, -) -> Option<(snippet::Snippet, usize)> { - let term = pattern.literal_folded()?; - Some(snippet::extract_folded(text, folded, term, &SNIPPET_OPTS)) + lower: &mut String, +) -> Option { + use crate::query::pattern::TermPattern; + match pattern { + TermPattern::Empty => None, + TermPattern::Literal(_) => { + let (typed, term) = (pattern.literal()?, pattern.literal_folded()?); + fold_into(lower, text); + let found = snippet::extract_folded(text, lower, term, typed, SNIPPET_WINDOW_BYTES)?; + let (stage, count) = match found.exact { + 0 => (6, found.count), + exact => (5, exact), + }; + Some(BodyMatch { + stage, + count, + snippet: found.snippet, + }) + } + TermPattern::Wildcard(w) => { + let (stage, (count, first)) = match w.count_and_first(text, false) { + (0, _) => (6, w.count_and_first(text, true)), + exact => (5, exact), + }; + // A greedy pattern can match megabytes; clamp before the window. + let r = clamp_match_range(text, first?, SNIPPET_WINDOW_BYTES); + Some(BodyMatch { + stage, + count, + snippet: snippet::window_around(text, (r.start, r.end), SNIPPET_WINDOW_BYTES), + }) + } + } +} + +/// Fold `text` into `dst`, reusing its allocation. The ASCII fold is +/// byte-length preserving, so folded offsets are valid in the original. +fn fold_into(dst: &mut String, text: &str) { + dst.clear(); + dst.push_str(text); + dst.make_ascii_lowercase(); } /// The fuzzy full-text match in one document body: occurrence count within @@ -92,7 +125,7 @@ pub fn fuzzy_snippet( text: &str, ) -> Option<(usize, snippet::Snippet)> { let (count, first) = bitap.count_and_first(text.as_bytes()); - first.map(|range| (count, snippet::window_around(text, range, &SNIPPET_OPTS))) + first.map(|range| (count, snippet::window_around(text, range, SNIPPET_WINDOW_BYTES))) } #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] @@ -465,13 +498,20 @@ impl<'a> Cx<'a> { self.flush_pass(deferred.hits, deferred.overflowed); } - /// Sort + cut back to display-limit room once past the cap; true if anything dropped. + /// Cut a deferred buffer back to display-limit room once past the cap; + /// true if anything dropped. Partitioned, not sorted: [`Cx::flush_pass`] + /// sorts what is left. Measured against a full sort (`searchtime`, cargo + /// registry, terms every path holds): `1949cf` 25–30 → 21–22 ms, `src` + /// 40–42 → 37–39, `crates` 89–99 → 86–87. fn enforce_cap(&self, buf: &mut Vec) -> bool { if buf.len() <= self.buffer_cap() { return false; } - buf.sort_by(rank_order); - buf.truncate(self.remaining()); + let keep = self.remaining(); + if keep > 0 { + buf.select_nth_unstable_by(keep - 1, rank_order); + } + buf.truncate(keep); true } } diff --git a/crates/quicksearch-core/src/search/cascade/passes.rs b/crates/quicksearch-core/src/search/cascade/passes.rs index d1edfe5..496df61 100644 --- a/crates/quicksearch-core/src/search/cascade/passes.rs +++ b/crates/quicksearch-core/src/search/cascade/passes.rs @@ -84,14 +84,6 @@ fn fts_match_sql(filter_sql: &str) -> String { ) } -/// Fold `text` into `dst`, reusing its allocation. The ASCII fold is -/// byte-length preserving, so folded offsets are valid in the original. -fn fold_into(dst: &mut String, text: &str) { - dst.clear(); - dst.push_str(text); - dst.make_ascii_lowercase(); -} - /// The segment pass A's path-tier prefilter runs on: the longest one /// containing no path separator, or `None` when every segment has one. Any /// single segment is a superset test, and a separator-free one cannot span @@ -157,7 +149,6 @@ impl<'a> Cx<'a> { .map_err(|e| e.to_string())?; let mut buf: Vec = Vec::new(); - let mut overflowed = false; let mut deferred = Deferred::default(); let mut scanned = 0usize; let mut clock = FlushClock::new(); @@ -189,8 +180,9 @@ impl<'a> Cx<'a> { match classify(self, row, file_id, &path, &path[name_at..])? { RowHit::Skip => {} RowHit::Emit(hit) => { + // No cap here: `flush_if_due` hands the buffer over at + // every `batch` rows, so only deferred hits accumulate. buf.push(hit); - overflowed |= self.enforce_cap(&mut buf); self.flush_if_due(&mut buf, &mut clock); // Stop once the display limit is full. Recorded, not // turned into `limited` here: `flush_pass` owns that call. @@ -213,7 +205,7 @@ impl<'a> Cx<'a> { Some(slot) => *self.deferred_mut(slot) = deferred, None => debug_assert!(deferred.hits.is_empty(), "deferred hits with no slot"), } - self.flush_pass(buf, overflowed); + self.flush_pass(buf, false); // After the flush, so it cannot be undone by one that truncated nothing; // without this a cut set can report itself complete. self.limited |= cut_short; @@ -329,41 +321,24 @@ impl<'a> Cx<'a> { let Some(text) = stored_text(row, &mut doc)? else { return Ok(RowHit::Skip); }; - // Fold once: count, first match and snippet all need it, even on - // rows about to be dropped. - fold_into(&mut lower, text); - let (count, stage, snip) = match super::text_snippet_counted(pattern, text, &lower) { - Some((snip, count_ci)) => { - if count_ci == 0 { - return Ok(RowHit::Skip); - } - let count_cs = pattern.count(text); - if count_cs > 0 { - (count_cs, 5, Some(snip)) - } else { - (count_ci, 6, Some(snip)) - } - } - None => { - let count_cs = pattern.count(text); - let (count, stage) = if count_cs > 0 { - (count_cs, 5) - } else { - let count_ci = pattern.count_folded(&lower); - if count_ci == 0 { - return Ok(RowHit::Skip); - } - (count_ci, 6) - }; - (count, stage, super::text_snippet(pattern, text, &lower)) - } + let Some(m) = super::body_match(pattern, text, &mut lower) else { + return Ok(RowHit::Skip); }; - let rank = stage as f64 + count_frac(count); + let rank = m.stage as f64 + count_frac(m.count); if !cx.regex_accepts(file_id, path, Some(text))? { return Ok(RowHit::Skip); } - row_hit(row, file_id, path, name, rank, stage, snip, false) + row_hit( + row, + file_id, + path, + name, + rank, + m.stage, + Some(m.snippet), + false, + ) }) } @@ -529,15 +504,12 @@ impl<'a> Cx<'a> { let Some(text) = stored_text(row, &mut doc)? else { return Ok(RowHit::Skip); }; - let count = re.count(text); - if count == 0 { + let (count, Some(first)) = re.count_and_first(text) else { return Ok(RowHit::Skip); - } + }; // A greedy user regex can match megabytes; clamp before the window. - let snip = re.find_first(text).map(|r| { - let r = clamp_match_range(text, r, SNIPPET_WINDOW_BYTES); - snippet::window_around(text, (r.start, r.end), &SNIPPET_OPTS) - }); + let r = clamp_match_range(text, first, SNIPPET_WINDOW_BYTES); + let snip = snippet::window_around(text, (r.start, r.end), SNIPPET_WINDOW_BYTES); row_hit( row, file_id, @@ -545,7 +517,7 @@ impl<'a> Cx<'a> { name, 6.0 + count_frac(count), 6, - snip, + Some(snip), false, ) }) diff --git a/crates/quicksearch-core/src/search/duplicates.rs b/crates/quicksearch-core/src/search/duplicates.rs index cca7fb1..af9e360 100644 --- a/crates/quicksearch-core/src/search/duplicates.rs +++ b/crates/quicksearch-core/src/search/duplicates.rs @@ -15,6 +15,10 @@ //! database in RAM (the gap widens on cold or encrypted storage). An innocent //! edit can lose this, so a test reads the query plan back and fails on //! anything but a covering scan. +//! +//! The index exists only once a run has finished (see +//! [`crate::db::repo::build_hash_index`]); before that the same query sorts +//! the table, slower but with the same answer. use std::cmp::Ordering; use std::collections::BinaryHeap; @@ -239,6 +243,8 @@ mod tests { .expect("unique path"); } tx.commit().unwrap(); + // As a finished run's maintenance leaves it. + crate::db::repo::build_hash_index(&conn).unwrap(); (scratch, p) } @@ -282,6 +288,28 @@ mod tests { assert_eq!(groups[1].members[0].1, "a1.txt", "members path-ordered"); } + /// A fresh index has no hash index until its first run's maintenance + /// builds one; the listing must not depend on it, only get faster. + #[test] + fn groups_are_the_same_before_the_hash_index_is_built() { + let (_dir, p) = seed_db(); + let indexed = find_duplicate_groups(&p, 10).unwrap(); + let conn = db::open_existing(&p, true).unwrap(); + conn.execute_batch("DROP INDEX idx_files_hash;").unwrap(); + assert_eq!(find_duplicate_groups(&p, 10).unwrap(), indexed); + + crate::db::repo::maintain(&conn, &p).unwrap(); + let rebuilt: i64 = conn + .query_row( + "SELECT COUNT(*) FROM sqlite_master WHERE name = 'idx_files_hash'", + [], + |r| r.get(0), + ) + .unwrap(); + assert_eq!(rebuilt, 1, "maintenance builds the missing index"); + assert_eq!(find_duplicate_groups(&p, 10).unwrap(), indexed); + } + #[test] fn limit_keeps_the_largest_groups() { let (_dir, p) = seed_db(); diff --git a/crates/quicksearch-core/src/search/fuzzy.rs b/crates/quicksearch-core/src/search/fuzzy.rs index ef29f52..f1dc392 100644 --- a/crates/quicksearch-core/src/search/fuzzy.rs +++ b/crates/quicksearch-core/src/search/fuzzy.rs @@ -6,12 +6,18 @@ //! byte are set), so neither side is folded. Measured: folding the haystack //! cost a copy per scanned field and per stored document. +use std::ops::ControlFlow; + use crate::query::split::CascadeQuery; /// Registers: one per error count `0..=k`. [`edit_budget`]'s ladder against a /// 64-byte pattern caps `k` at 21; [`Bitap::new`] rejects more. const MAX_REGISTERS: usize = 22; +/// [`Bitap::step`]'s register count, taken from `k` at run time rather than +/// fixed by [`Bitap::each_match`]'s dispatch. +const RUNTIME_K: usize = usize::MAX; + pub struct Bitap { /// `masks[c]` has bit `i` set iff `pattern[i] == c` ignoring ASCII case. masks: [u64; 256], @@ -19,6 +25,8 @@ pub struct Bitap { rev_masks: [u64; 256], len: usize, k: usize, + /// The bit that says the whole pattern has matched: `1 << (len - 1)`. + done: u64, } impl Bitap { @@ -44,6 +52,7 @@ impl Bitap { rev_masks, len: pattern.len(), k, + done: 1 << (pattern.len() - 1), }) } @@ -56,31 +65,75 @@ impl Bitap { } } - /// Advance all registers by one haystack byte; returns the smallest error - /// count d at which the full pattern just matched. `masks` selects the - /// scan direction: [`Bitap::masks`] forwards, [`Bitap::rev_masks`] back. - #[inline] - fn step(&self, masks: &[u64; 256], r: &mut [u64], byte: u8) -> Option { + /// Advance all registers by one haystack byte; `true` when the full + /// pattern just matched within `k` errors. `masks` selects the scan + /// direction: [`Bitap::masks`] forwards, [`Bitap::rev_masks`] back. + /// + /// Registers only grow with the error count (`r[d] ⊇ r[d-1]`, from + /// `reset` on by induction), so `r[k]` alone answers "matched", and + /// [`Bitap::errors`] finds how well only when it did. Measured against + /// testing every register every byte (`searchtime`, cargo registry, 41k + /// files): `~recieve` 361 → 310 ms, `~serialze` 453 → 397, `~HashMpa` + /// 569 → 517. + #[inline(always)] + fn step( + &self, + masks: &[u64; 256], + r: &mut [u64; MAX_REGISTERS], + byte: u8, + ) -> bool { + let k = if K == RUNTIME_K { self.k } else { K }; let mask = masks[byte as usize]; - let done = 1u64 << (self.len - 1); - let mut hit = None; let mut prev_old = r[0]; // R_old[d-1] for the d-th iteration r[0] = ((r[0] << 1) | 1) & mask; - if r[0] & done != 0 { - hit = Some(0); - } - for d in 1..=self.k { + for d in 1..=k { let old = r[d]; r[d] = (((old << 1) | 1) & mask) // extend a ≤d-error state | prev_old // insertion in text | (prev_old << 1) // substitution | ((r[d - 1] << 1) | 1); // deletion (pattern byte skipped) prev_old = old; - if hit.is_none() && r[d] & done != 0 { - hit = Some(d); + } + r[k] & self.done != 0 + } + + /// Call `hit` with the end of every match a forward scan finds and the + /// registers there, which it may reset; stop when it breaks. The budgets + /// default searches use get a copy with the register count fixed, so the + /// loop unrolls. Measured against the runtime count, on top of the note + /// at [`Bitap::step`]: `~recieve` 307–322 → 195–199 ms, `~serialze` + /// 391–398 → 239–240, `~HashMpa` 497–505 → 302–304. + fn each_match( + &self, + hay: &[u8], + hit: impl FnMut(usize, &mut [u64; MAX_REGISTERS]) -> ControlFlow<()>, + ) { + match self.k { + 1 => self.scan::<1>(hay, hit), + 2 => self.scan::<2>(hay, hit), + _ => self.scan::(hay, hit), + } + } + + fn scan( + &self, + hay: &[u8], + mut hit: impl FnMut(usize, &mut [u64; MAX_REGISTERS]) -> ControlFlow<()>, + ) { + let mut r = [0u64; MAX_REGISTERS]; + self.reset(&mut r); + for (i, &b) in hay.iter().enumerate() { + if self.step::(&self.masks, &mut r, b) && hit(i + 1, &mut r).is_break() { + return; } } - hit + } + + /// The fewest errors the match [`Bitap::step`] just reported has. + fn errors(&self, r: &[u64; MAX_REGISTERS]) -> usize { + (0..=self.k) + .find(|&d| r[d] & self.done != 0) + .unwrap_or(self.k) } /// Where the match that ended at `end` with `errors` edits began: the same @@ -95,10 +148,8 @@ impl Bitap { let mut r = [0u64; MAX_REGISTERS]; self.reset(&mut r); for (back, &b) in hay[floor..end].iter().rev().enumerate() { - if self - .step(&self.rev_masks, &mut r, b) - .is_some_and(|d| d <= errors) - { + self.step::(&self.rev_masks, &mut r, b); + if r[errors] & self.done != 0 { return end - (back + 1); } } @@ -121,7 +172,8 @@ impl Bitap { let mut best = (errors, end); let mut probe = *r; for (ahead, &b) in hay[end..].iter().take(errors).enumerate() { - if let Some(d) = self.step(&self.masks, &mut probe, b) { + if self.step::(&self.masks, &mut probe, b) { + let d = self.errors(&probe); if d < best.0 { best = (d, end + ahead + 1); } @@ -133,21 +185,18 @@ impl Bitap { /// The smallest edit distance (≤ k) at which the pattern occurs in `hay`, /// and that occurrence's byte range: the span a frontend marks. pub fn best_distance_and_first(&self, hay: &[u8]) -> Option<(usize, (usize, usize))> { - let mut r = [0u64; MAX_REGISTERS]; - self.reset(&mut r); let mut best: Option<(usize, usize)> = None; - for (i, &b) in hay.iter().enumerate() { - if let Some(d) = self.step(&self.masks, &mut r, b) { - let end = i + 1; - if d == 0 { - best = Some((0, end)); - break; - } - if best.is_none_or(|(cur, _)| d < cur) { - best = Some((d, end)); - } + self.each_match(hay, |end, r| { + let d = self.errors(r); + if best.is_none_or(|(cur, _)| d < cur) { + best = Some((d, end)); } - } + if d == 0 { + ControlFlow::Break(()) + } else { + ControlFlow::Continue(()) + } + }); best.map(|(d, end)| (d, (self.match_start(hay, end, d), end))) } @@ -155,20 +204,17 @@ impl Bitap { /// one's byte range in `hay`. After each hit the automaton resets, so /// overlapping suffix matches don't inflate counts. pub fn count_and_first(&self, hay: &[u8]) -> (usize, Option<(usize, usize)>) { - let mut r = [0u64; MAX_REGISTERS]; - self.reset(&mut r); let mut count = 0usize; let mut first: Option<(usize, usize)> = None; - for (i, &b) in hay.iter().enumerate() { - if let Some(d) = self.step(&self.masks, &mut r, b) { - count += 1; - if first.is_none() { - let (errors, end) = self.refine_end(hay, &r, i + 1, d); - first = Some((self.match_start(hay, end, errors), end)); - } - self.reset(&mut r); + self.each_match(hay, |end, r| { + count += 1; + if first.is_none() { + let (errors, end) = self.refine_end(hay, r, end, self.errors(r)); + first = Some((self.match_start(hay, end, errors), end)); } - } + self.reset(r); + ControlFlow::Continue(()) + }); (count, first) } } @@ -517,6 +563,35 @@ mod tests { } } + /// The budgets with a fixed register count must report exactly the + /// matches, and the errors at each, that the runtime-count loop does. + #[test] + fn fixed_register_counts_agree_with_the_runtime_loop() { + fn matches(b: &Bitap, hay: &[u8], fixed: bool) -> Vec<(usize, usize)> { + let mut out = Vec::new(); + let hit = |end, r: &mut [u64; MAX_REGISTERS]| { + out.push((end, b.errors(r))); + b.reset(r); + ControlFlow::Continue(()) + }; + if fixed { + b.each_match(hay, hit); + } else { + b.scan::(hay, hit); + } + out + } + let hay = b"the quick brown fox jumps over the lazy dog; The Quikc Browm Fx"; + for pattern in ["quick", "brown", "fox", "lazy dog", "jumps", "QUICK"] { + for k in 1..=2 { + let b = Bitap::new(pattern.as_bytes(), k).unwrap(); + let fixed = matches(&b, hay, true); + assert!(!fixed.is_empty(), "{pattern} occurs at k={k}"); + assert_eq!(fixed, matches(&b, hay, false), "{pattern} k={k}"); + } + } + } + #[test] fn matches_brute_force_oracle() { let mut seed: u64 = 0x2545F4914F6CDD1D; diff --git a/crates/quicksearch-core/src/search/mod.rs b/crates/quicksearch-core/src/search/mod.rs index 7886ffd..664494f 100644 --- a/crates/quicksearch-core/src/search/mod.rs +++ b/crates/quicksearch-core/src/search/mod.rs @@ -100,9 +100,6 @@ impl SearchHit { #[derive(Debug, Clone)] pub enum SearchUpdate { - Started { - generation: u64, - }, Hits { generation: u64, hits: Vec, @@ -121,8 +118,7 @@ pub enum SearchUpdate { impl SearchUpdate { pub fn generation(&self) -> u64 { match self { - SearchUpdate::Started { generation } - | SearchUpdate::Hits { generation, .. } + SearchUpdate::Hits { generation, .. } | SearchUpdate::Completed { generation, .. } | SearchUpdate::Error { generation, .. } => *generation, } @@ -409,7 +405,6 @@ impl Worker { fn handle(&mut self, req: SearchRequest) { let generation = req.generation; - self.send(SearchUpdate::Started { generation }); match self.search(&req) { Ok(Some(Outcome { total, limited })) => self.send(SearchUpdate::Completed { generation, diff --git a/crates/quicksearch-core/src/snippet.rs b/crates/quicksearch-core/src/snippet.rs index 6037da3..fd0f5de 100644 --- a/crates/quicksearch-core/src/snippet.rs +++ b/crates/quicksearch-core/src/snippet.rs @@ -3,19 +3,6 @@ //! markup. Matching folds ASCII case only, as full-text verification does: //! `cafe` neither finds nor marks `café`. -#[derive(Debug, Clone)] -pub struct Options { - /// Approximate byte budget for the returned window: a soft target, since - /// matches expand the window so a hit is never cut off. - pub approx_bytes: usize, -} - -impl Default for Options { - fn default() -> Self { - Options { approx_bytes: 200 } - } -} - /// A context window from a document plus the match positions inside it. #[derive(Debug, Clone, PartialEq, Eq)] pub struct Snippet { @@ -40,35 +27,46 @@ impl Snippet { } } -/// Extract against a haystack the caller has already ASCII-folded. `folded` -/// must be `text.to_ascii_lowercase()`: the fold preserves byte length, so -/// offsets found in it slice the original. +/// What [`extract_folded`] found in one body. +#[derive(Debug, Clone)] +pub struct Found { + pub snippet: Snippet, + /// Occurrences in any ASCII case, counted before touching ones were + /// coalesced: what the full-text pass ranks on. + pub count: usize, + /// How many of those are spelled exactly as typed. + pub exact: usize, +} + +/// Find `term` in a haystack the caller has already ASCII-folded, and cut the +/// window around its first occurrence. `folded` must be +/// `text.to_ascii_lowercase()` and `term` must be `typed` ASCII-lowercased: +/// the fold preserves byte length, so an offset found in `folded` slices +/// `text`, where the occurrence is compared with `typed`. `None` when the +/// term does not occur; a blank term never does, but a padded one is searched +/// as given, never trimmed. /// -/// Returns the window and how many occurrences of `term` it found before -/// touching ranges were coalesced, the count the full-text pass ranks on. A -/// blank term matches nothing, but a padded term is searched as given, never -/// trimmed. -pub fn extract_folded(text: &str, folded: &str, term: &str, opts: &Options) -> (Snippet, usize) { +/// `exact` comes from the case-insensitive occurrences rather than a second, +/// case-sensitive search: the same count for any term that cannot overlap +/// itself, and at most one short per overlap for one that can (`aA` in `aaA`). +pub fn extract_folded( + text: &str, + folded: &str, + term: &str, + typed: &str, + budget: usize, +) -> Option { debug_assert_eq!(folded.len(), text.len(), "ASCII folding preserves length"); - if text.is_empty() { - return (Snippet::empty(), 0); - } + debug_assert_eq!(term, typed.to_ascii_lowercase(), "`term` is `typed` folded"); if term.trim().is_empty() { - return (head_window(text, opts.approx_bytes), 0); + return None; } - // Borrow when the term is already folded, as every cascade call's is, - // to avoid an allocation per candidate row. - let pattern: std::borrow::Cow<'_, str> = if term.bytes().any(|b| b.is_ascii_uppercase()) { - std::borrow::Cow::Owned(term.to_ascii_lowercase()) - } else { - std::borrow::Cow::Borrowed(term) - }; // `memmem`, not `str::match_indices`: measured faster on whole-body - // scans. The walk runs to the end (the count must see every occurrence), + // scans. The walk runs to the end (the counts must see every occurrence), // but matches past the window's right edge are not stored. let mut matches: Vec<(usize, usize)> = Vec::new(); - let mut found = 0usize; + let (mut count, mut exact) = (0usize, 0usize); // The window's right edge once the first match fixes it, from the same // [`window_bounds`] the window itself uses. let mut keep_below: Option = None; @@ -76,29 +74,32 @@ pub fn extract_folded(text: &str, folded: &str, term: &str, opts: &Options) -> ( // (`abab…` for `ab`) that starts inside the window and continues past // it stays intact for `coalesce_overlapping` and the expansion step. let mut chain_end = 0usize; - for at in memchr::memmem::find_iter(folded.as_bytes(), pattern.as_bytes()) { + for at in memchr::memmem::find_iter(folded.as_bytes(), term.as_bytes()) { // Before any dropping: two abutting occurrences are two hits for // ranking even though they paint as one highlight. - found += 1; - let bound = *keep_below.get_or_insert_with(|| window_bounds(text, at, opts.approx_bytes).1); + count += 1; + if text.as_bytes()[at..at + term.len()] == *typed.as_bytes() { + exact += 1; + } + let bound = *keep_below.get_or_insert_with(|| window_bounds(text, at, budget).1); if at >= bound && at > chain_end { // Past the edge and not chained to anything kept: keep counting, // stop storing. continue; } - chain_end = at + pattern.len(); + chain_end = at + term.len(); matches.push((at, chain_end)); } if matches.is_empty() { - return (head_window(text, opts.approx_bytes), 0); + return None; } // One term's occurrences arrive in order and never overlap; only // abutting ones merge. let matches = coalesce_overlapping(matches); - let (win_start, mut win_end) = window_bounds(text, matches[0].0, opts.approx_bytes); + let (win_start, mut win_end) = window_bounds(text, matches[0].0, budget); // A match straddling the right edge is fully included, not cut mid-hit. if let Some((_, end)) = matches.iter().rfind(|(s, _)| *s < win_end) { if *end > win_end { @@ -117,15 +118,16 @@ pub fn extract_folded(text: &str, folded: &str, term: &str, opts: &Options) -> ( }) .collect(); - ( - Snippet { + Some(Found { + snippet: Snippet { window: text[win_start..win_end].to_string(), ranges, truncated_start: win_start > 0, truncated_end: win_end < text.len(), }, - found, - ) + count, + exact, + }) } /// The window cut around a match that starts at `first`: from a third of @@ -174,12 +176,12 @@ pub fn whole_field(text: &str, range: (usize, usize)) -> Snippet { /// Build a snippet window around one known match range in `text`. The range /// is clamped into `text` and widened to char boundaries. -pub fn window_around(text: &str, range: (usize, usize), opts: &Options) -> Snippet { +pub fn window_around(text: &str, range: (usize, usize), budget: usize) -> Snippet { if text.is_empty() { return Snippet::empty(); } let (ms, me) = aligned_range(text, range); - let (win_start, win_end) = window_bounds(text, ms, opts.approx_bytes); + let (win_start, win_end) = window_bounds(text, ms, budget); // The whole match stays inside the window. let win_end = win_end.max(me); @@ -196,26 +198,6 @@ pub fn window_around(text: &str, range: (usize, usize), opts: &Options) -> Snipp } } -/// Count non-overlapping occurrences of `term` in `text`, byte for byte: a -/// case-insensitive count passes both already folded. -pub fn count_occurrences(text: &str, term: &str) -> usize { - if term.is_empty() || text.len() < term.len() { - return 0; - } - memchr::memmem::find_iter(text.as_bytes(), term.as_bytes()).count() -} - -/// The first `n` bytes of `text` (char-aligned) as a match-less window. -fn head_window(text: &str, n: usize) -> Snippet { - let cut = text.floor_char_boundary(n); - Snippet { - window: text[..cut].to_string(), - ranges: Vec::new(), - truncated_start: false, - truncated_end: cut < text.len(), - } -} - /// Merge adjacent/overlapping ranges. Input must be sorted by start. fn coalesce_overlapping(v: Vec<(usize, usize)>) -> Vec<(usize, usize)> { let mut out = Vec::with_capacity(v.len()); @@ -239,80 +221,87 @@ fn coalesce_overlapping(v: Vec<(usize, usize)>) -> Vec<(usize, usize)> { mod tests { use super::*; - fn extract(text: &str, term: &str, opts: &Options) -> Snippet { - extract_folded(text, &text.to_ascii_lowercase(), term, opts).0 + /// [`extract_folded`] with the folding done here; `typed` as given. + fn find(text: &str, typed: &str, budget: usize) -> Option { + let term = typed.to_ascii_lowercase(); + extract_folded(text, &text.to_ascii_lowercase(), &term, typed, budget) } - fn extract_counted(text: &str, term: &str, opts: &Options) -> (Snippet, usize) { - extract_folded(text, &text.to_ascii_lowercase(), term, opts) + fn extract(text: &str, term: &str, budget: usize) -> Snippet { + find(text, term, budget).expect("the term occurs").snippet } - fn opts_small() -> Options { - Options { approx_bytes: 40 } + fn extract_counted(text: &str, term: &str, budget: usize) -> (Snippet, usize) { + let found = find(text, term, budget).expect("the term occurs"); + (found.snippet, found.count) } + /// Non-overlapping occurrences, byte for byte. + fn memmem_count(text: &str, term: &str) -> usize { + memchr::memmem::find_iter(text.as_bytes(), term.as_bytes()).count() + } + + /// A window budget small enough that tests see truncation. + const SMALL: usize = 40; + /// The count is occurrences, not the highlights they coalesce into. #[test] fn extract_folded_counts_occurrences_not_ranges() { let text = "abab and ab"; - let (s, n) = extract_counted(text, "ab", &opts_small()); + let (s, n) = extract_counted(text, "ab", SMALL); assert_eq!(n, 3, "three occurrences"); - assert_eq!( - n, - count_occurrences(text, "ab"), - "must agree with count_occurrences" - ); + assert_eq!(n, memmem_count(text, "ab")); assert_eq!(s.ranges.len(), 2, "the abutting pair paints as one range"); } - /// A disagreement here would change which rows survive the pass. + /// A disagreement here would change which rows survive the pass, or + /// rank them: both counts must match separate searches, one over the + /// folded body and one over the body as stored. #[test] - fn extract_folded_count_agrees_with_count_occurrences() { + fn extract_folded_counts_agree_with_separate_searches() { let cases: &[(&str, &str)] = &[ - ("no hits at all", "zzz"), ("one hit here", "hit"), ("Hit hit HIT", "hit"), + ("Hit hit HIT", "HIT"), + ("Hit hit HIT", "hIT"), ("aaaa", "aa"), - ("", "x"), - ("short", "much longer than the haystack"), - ("ünïcode ünïcode", "ünïcode"), + ("ünïcode ÜNÏCODE ünïcode", "ünïcode"), + ("Needle and needle and NEEDLE", "Needle"), ]; - for (text, term) in cases { + for (text, typed) in cases { + let found = find(text, typed, SMALL).expect("the term occurs"); let folded = text.to_ascii_lowercase(); - let (_, n) = extract_folded(text, &folded, term, &opts_small()); + let term = typed.to_ascii_lowercase(); assert_eq!( - n, - count_occurrences(&folded, &term.to_ascii_lowercase()), - "count mismatch for {:?} in {:?}", - term, - text + found.count, + memmem_count(&folded, &term), + "{typed:?} in {text:?}" + ); + assert_eq!( + found.exact, + memmem_count(text, typed), + "{typed:?} in {text:?}" ); } } - /// The owning and borrowing branches of the fold must find the same - /// matches. #[test] - fn extract_folded_needle_case_does_not_change_matches() { - let text = "The Needle and the needle"; - let folded = text.to_ascii_lowercase(); - let (upper, n_upper) = extract_folded(text, &folded, "Needle", &opts_small()); - let (lower, n_lower) = extract_folded(text, &folded, "needle", &opts_small()); - assert_eq!(n_upper, 2); - assert_eq!(n_upper, n_lower); - assert_eq!(upper.ranges, lower.ranges); - assert_eq!(upper.window, lower.window); + fn a_term_that_does_not_occur_finds_nothing() { + for (text, term) in [ + ("no hits at all", "zzz"), + ("", "x"), + ("short", "much longer than the haystack"), + ("alpha beta gamma", "nomatch"), + ] { + assert!(find(text, term, SMALL).is_none(), "{term:?} in {text:?}"); + } } /// An empty needle would match at every byte offset. #[test] fn extract_folded_ignores_blank_terms() { - let text = "some text"; - let folded = text.to_ascii_lowercase(); for term in ["", " ", "\t"] { - let (s, n) = extract_folded(text, &folded, term, &opts_small()); - assert_eq!(n, 0, "blank term {:?} counted", term); - assert!(s.ranges.is_empty(), "blank term {:?} highlighted", term); + assert!(find("some text", term, SMALL).is_none(), "{term:?}"); } } @@ -320,10 +309,8 @@ mod tests { #[test] fn extract_folded_does_not_trim_a_padded_term() { let text = "needle needlework"; - let folded = text.to_ascii_lowercase(); - let (_, n) = extract_folded(text, &folded, "needle ", &opts_small()); + let (_, n) = extract_counted(text, "needle ", SMALL); assert_eq!(n, 1, "only the occurrence followed by a space"); - assert_eq!(n, count_occurrences(&folded, "needle ")); } /// The `Snippet::ranges` contract egui's LayoutJob sections rely on. @@ -349,7 +336,7 @@ mod tests { fn occurrences_past_the_window_are_counted_but_not_kept() { let unit = "needle filler filler "; let text = unit.repeat(400); - let (snip, found) = extract_counted(&text, "needle", &opts_small()); + let (snip, found) = extract_counted(&text, "needle", SMALL); assert_eq!(found, 400, "every occurrence must still be counted"); assert_ranges_valid(&snip); @@ -378,7 +365,7 @@ mod tests { #[test] fn a_coalescing_chain_is_not_cut_at_the_window_edge() { let text = "ab".repeat(400); - let (snip, found) = extract_counted(&text, "ab", &opts_small()); + let (snip, found) = extract_counted(&text, "ab", SMALL); assert_eq!(found, 400, "non-overlapping occurrences, all counted"); assert_ranges_valid(&snip); @@ -403,46 +390,23 @@ mod tests { ); } - #[test] - fn empty_text_returns_empty() { - let s = extract("", "foo", &Options::default()); - assert_eq!(s, Snippet::empty()); - } - - #[test] - fn a_blank_term_returns_head_marked_truncated() { - let long = "abcdefghijklmnop".repeat(10); - let s = extract(&long, "", &opts_small()); - assert!(s.truncated_end); - assert!(!s.truncated_start); - assert!(s.ranges.is_empty()); - assert!(s.window.len() <= 40); - } - - #[test] - fn a_blank_term_untruncated() { - let s = extract("short text", "", &opts_small()); - assert_eq!(s.window, "short text"); - assert!(!s.truncated_end && !s.truncated_start); - } - #[test] fn simple_match_range() { - let s = extract("the quick brown fox", "quick", &opts_small()); + let s = extract("the quick brown fox", "quick", SMALL); assert_eq!(marked(&s), vec!["quick"]); assert_ranges_valid(&s); } #[test] fn case_insensitive_match_reports_original_case() { - let s = extract("The QUICK brown fox", "quick", &opts_small()); + let s = extract("The QUICK brown fox", "quick", SMALL); assert_eq!(marked(&s), vec!["QUICK"]); } #[test] fn window_truncation_flags_on_both_sides() { let text = "prefix ".repeat(20) + "MATCH in middle " + &"suffix ".repeat(20); - let s = extract(&text, "MATCH", &opts_small()); + let s = extract(&text, "MATCH", SMALL); assert!(s.truncated_start); assert!(s.truncated_end); assert_eq!(marked(&s), vec!["MATCH"]); @@ -450,27 +414,15 @@ mod tests { #[test] fn match_at_start_not_truncated_left() { - let s = extract( - "MATCH right at the start of this paragraph", - "match", - &opts_small(), - ); + let s = extract("MATCH right at the start of this paragraph", "match", SMALL); assert!(!s.truncated_start); assert_eq!(s.ranges[0].0, 0); } - #[test] - fn no_match_returns_head_without_ranges() { - let text = "alpha beta gamma delta epsilon zeta eta theta iota kappa"; - let s = extract(text, "nomatch", &opts_small()); - assert!(s.ranges.is_empty()); - assert!(s.window.starts_with("alpha")); - } - #[test] fn utf8_boundaries_hold_with_multibyte_text() { let text = "café café café café café café café café café café"; - let s = extract(text, "café", &opts_small()); + let s = extract(text, "café", SMALL); assert!(!s.ranges.is_empty()); assert_ranges_valid(&s); for m in marked(&s) { @@ -482,7 +434,7 @@ mod tests { fn match_near_right_edge_is_fully_included() { let prefix = "x".repeat(30); let text = format!("{}{}", prefix, "LONGMATCHTERMTEXT"); - let s = extract(&text, "LONGMATCHTERMTEXT", &opts_small()); + let s = extract(&text, "LONGMATCHTERMTEXT", SMALL); assert_eq!(marked(&s), vec!["LONGMATCHTERMTEXT"]); } @@ -490,7 +442,7 @@ mod tests { fn window_around_basic() { let text = "prefix ".repeat(20) + "NEEDLE" + &" suffix".repeat(20); let at = text.find("NEEDLE").unwrap(); - let s = window_around(&text, (at, at + 6), &opts_small()); + let s = window_around(&text, (at, at + 6), SMALL); assert_eq!(marked(&s), vec!["NEEDLE"]); assert!(s.truncated_start && s.truncated_end); assert_ranges_valid(&s); @@ -498,28 +450,18 @@ mod tests { #[test] fn window_around_clamps_out_of_bounds() { - let s = window_around("tiny", (2, 999), &opts_small()); + let s = window_around("tiny", (2, 999), SMALL); assert_eq!(s.window, "tiny"); assert_eq!(s.ranges, vec![(2, 4)]); // Fully out-of-range → no ranges, but never a panic. - let s = window_around("tiny", (999, 1000), &opts_small()); + let s = window_around("tiny", (999, 1000), SMALL); assert!(s.ranges.is_empty()); } #[test] fn window_around_aligns_multibyte_boundaries() { let text = "ééééééééé needle ééééééééé"; - let s = window_around(text, (1, 3), &opts_small()); + let s = window_around(text, (1, 3), SMALL); assert_ranges_valid(&s); } - - #[test] - fn count_occurrences_cases() { - assert_eq!(count_occurrences("aaaa", "aaa"), 1, "non-overlapping"); - assert_eq!(count_occurrences("abcABC", "abc"), 1, "byte for byte"); - assert_eq!(count_occurrences("", "x"), 0); - assert_eq!(count_occurrences("xyz", ""), 0); - assert_eq!(count_occurrences("no hits here", "zzz"), 0); - assert_eq!(count_occurrences("ab ab ab", "ab"), 3); - } } diff --git a/crates/quicksearch-core/src/testutil.rs b/crates/quicksearch-core/src/testutil.rs index b577177..6353b95 100644 --- a/crates/quicksearch-core/src/testutil.rs +++ b/crates/quicksearch-core/src/testutil.rs @@ -452,6 +452,8 @@ pub fn seed_index(path: &std::path::Path, spec: &SeedSpec) { } } tx.commit().unwrap(); + // As a finished run's maintenance leaves it. + crate::db::repo::build_hash_index(&conn).unwrap(); conn.execute_batch("PRAGMA wal_checkpoint(TRUNCATE);").ok(); } diff --git a/crates/quicksearch-core/src/walk.rs b/crates/quicksearch-core/src/walk.rs index d23700b..24e022e 100644 --- a/crates/quicksearch-core/src/walk.rs +++ b/crates/quicksearch-core/src/walk.rs @@ -16,8 +16,8 @@ use std::thread::JoinHandle; use crate::config::{Config, IgnoreSet}; use crate::extract::Registry; use crate::file_handling::{ - classify_by_mtime, classify_for_indexing, dir_to_db_parent, path_to_db_string, - prepare_file_record, DirRows, FileIndexAction, OwnedNewFile, UnreadableDirs, + classify_by_mtime, dir_to_db_parent, path_to_db_string, prepare_file_record, DirRows, + FileIndexAction, OwnedNewFile, UnreadableDirs, }; mod pool; @@ -30,6 +30,10 @@ use pool::{Found, PrefetchWork, Queue, Shared}; /// Files one worker takes for itself before handing the rest to the pool. const FILES_PER_JOB: usize = 128; +/// Events a root's walk may run ahead of the writer. Each can carry up to +/// `hash_length` of inline text, but the run's peak is set during extraction: +/// measured at 512 (`memprobe cold`, `/usr`, 373k files), peak RSS 343–355 +/// against 293–309 MiB here and the walk no faster. Measured and rejected. const CHANNEL_CAP: usize = 4096; const LOCAL_THREADS: usize = 4; @@ -77,11 +81,14 @@ pub enum WalkEvent { struct PendingFile { path: PathBuf, cached: Option, + /// The stored mtime, taken out of the directory's rows as it was listed; + /// `None` when the file has no row yet. + stored: Option, } enum Job { - Dir(PathBuf, Arc), - Files(Vec, Arc), + Dir(PathBuf, DirRows), + Files(Vec), /// A resolved symlink target, with the stored mtime for its own path. Alias(PathBuf, Option), } @@ -146,9 +153,12 @@ pub fn reset_run_warnings() { /// for the pool, the remaining files come back for this worker, `stale` gets /// the paths whose row has no file behind it. A directory that cannot be read /// returns before reconciling: it must not read as an empty one. +/// +/// Each listed file takes its row out of `rows`, carrying the stored mtime +/// with it, so what is left afterwards is exactly the stale set. fn read_directory( dir: &Path, - rows: &Arc, + mut rows: DirRows, ctx: &Ctx, found: &mut Vec, stale: &mut Vec, @@ -169,17 +179,11 @@ fn read_directory( } }; // Every `continue` in the loop must be a genuine "not indexable", or the - // stale diff below deletes live rows. Only names that have a row matter to - // that diff, so `present` borrows them from `rows`: no allocation per - // entry, and none at all for a directory with no rows yet. Measured - // (`indexprobe cold`, 100k files): 8.2 → 7.2 allocations per file, - // churn 42 → 35 MiB; time did not move, being `stat`-bound. - let mut present: HashSet<&str> = HashSet::new(); - let mut mark_present = |name: &str| { - if let Some((known, _)) = rows.get_key_value(name) { - present.insert(known.as_str()); - } - }; + // stale diff below deletes live rows: only a name taken out of `rows` + // keeps its row. Measured against marking names present in a set borrowed + // from `rows` and looking each one up again in `prepare` (`indexprobe + // warm`, `/usr`, 373k files): 9.3–9.8 → 8.8–9.3 allocations per file, + // churn 246–257 → 231–240 MiB; time within noise, being `stat`-bound. let mut unreadable_entry = false; let mut files = Vec::new(); @@ -198,17 +202,22 @@ fn read_directory( } }; - let name = entry.file_name(); + // The name borrowed from the path, which the entry allocates either + // way: `file_name()` would allocate a second copy. With the path's + // buffer becoming its `String` in `prepare`, measured (`indexprobe + // warm`, `/usr`, 373k files): 9.2–9.6 → 6.6–6.9 allocations per + // file, churn 240–247 → 203–210 MiB, user CPU 0.92–1.01 → 0.83–0.92 s. + let path = entry.path(); // The walk's one screen for names the index cannot spell (invalid // UTF-8 on Unix, unpaired UTF-16 surrogates on Windows; see - // `path_to_db_string`), skipped by choice. Leaving the entry out of - // `present` is safe: no stored row can carry such a name. - let Some(name) = name.to_str() else { + // `path_to_db_string`), skipped by choice. Its row cannot exist: no + // stored row can carry such a name. + let Some(name) = path.file_name().and_then(|n| n.to_str()) else { if UNREPRESENTABLE_WARNINGS.allow() { crate::log_warn!( "Skipping {:?} (name is not valid UTF-8, so it cannot be stored, hashed \ or text-indexed)", - entry.path() + path ); } continue; @@ -233,7 +242,7 @@ fn read_directory( crate::log_info!( "skipping {}: hidden attribute set (enable \"include hidden \ files\" to index it)", - entry.path().display() + path.display() ); } continue; @@ -242,13 +251,12 @@ fn read_directory( } // Ignored, or the index's own database and sidecars (see // `file_handling::index_file_set`). `continue`, not `skipped`: the - // name stays out of `present`, so old rows for the index fall to the - // stale sweep; `skipped` would keep them. + // row stays in `rows`, so old rows for the index fall to the stale + // sweep; `skipped` would keep them. if ctx.ignore.matches_component(name) { ctx.pruned.ignored.fetch_add(1, Ordering::Relaxed); continue; } - let path = entry.path(); if ctx.ignore.matches_path_pattern(&path) || ctx.index_files.contains(&path) { ctx.pruned.ignored.fetch_add(1, Ordering::Relaxed); continue; @@ -256,13 +264,13 @@ fn read_directory( // `file_type` is the cached `d_type` from the directory read. match entry.file_type() { - // Not marked present: a name that was a file last run and is a + // Row left in `rows`: a name that was a file last run and is a // directory now should lose its row. Ok(ft) if ft.is_dir() => found.push(Found::Dir(path)), // Directory and file targets gate together, or the walkers // disagree: `filtered_walk` follows neither kind, so a file target - // followed only here would never update between runs. Never - // marked present: the target's row belongs to its own directory. + // followed only here would never update between runs. Row left + // in `rows`: the target's row belongs to its own directory. Ok(ft) if ft.is_symlink() => { if ctx.follow_symlinks { if let Some(target) = resolve_link(&path, ctx) { @@ -271,29 +279,31 @@ fn read_directory( } } Ok(_) => { - mark_present(name); + let stored = rows.remove(name); // `None` on Unix and on any reparse point. let cached = crate::platform::entry_cached_metadata(|| entry.metadata().ok()); - files.push(PendingFile { path, cached }); + files.push(PendingFile { + path, + cached, + stored, + }); + } + // Type unknown: taken out so an existing row survives. + Err(_) => { + rows.remove(name); } - // Type unknown: mark it present so an existing row survives. - Err(_) => mark_present(name), } } if !unreadable_entry { - stale.extend( - rows.keys() - .filter(|name| !present.contains(name.as_str())) - .map(|name| path_to_db_string(&dir.join(name))), - ); + stale.extend(rows.into_keys().map(|name| path_to_db_string(&dir.join(name)))); } // Spread a wide directory across the pool, keeping the tail for ourselves // so the entries the read just warmed are handled now. while files.len() > FILES_PER_JOB { let chunk = files.split_off(files.len() - FILES_PER_JOB); - found.push(Found::Files(chunk, rows.clone())); + found.push(Found::Files(chunk)); } files } @@ -333,38 +343,35 @@ fn resolve_link(path: &Path, ctx: &Ctx) -> Option { } } -/// How a file's stored mtime is to be found. -enum Known<'a> { - InDir(&'a DirRows), - /// Resolved by exact path: a symlink target's row lives under a - /// different parent. - Exact(Option), -} - /// At most one `stat`, then classify; only files that will be written get /// opened, and small text files are finished outright. "At most": on Windows /// [`PendingFile::cached`] may already hold the answer. /// -/// `scratch` is this worker's, carrying the head buffer every hash and MIME -/// sniff reads into. +/// `aliased`: reached by resolving a symlink, so its row lives under a +/// different parent than the link's. `scratch` is this worker's, carrying the +/// head buffer every hash and MIME sniff reads into. fn prepare( file: PendingFile, - known: Known<'_>, + aliased: bool, ctx: &Ctx, scratch: &mut crate::extract::Scratch, ) -> WalkedFile { - let PendingFile { path, cached } = file; - // Every route here has already screened the path for UTF-8; see - // `path_to_db_string`. - debug_assert!( - path.to_str().is_some(), - "an unrepresentable path reached prepare(): {:?}", - path - ); - let db_path = path_to_db_string(&path); - let aliased = matches!(known, Known::Exact(_)); + let PendingFile { + path, + cached, + stored, + } = file; + // Every route here has already screened the path for UTF-8, and every + // walk path is a normalized root or link target with names joined on, so + // it already is its stored spelling: the buffer becomes the `String` + // without a copy (see `path_to_db_string`). + let db_path = path.into_os_string().into_string().unwrap_or_else(|path| { + debug_assert!(false, "an unrepresentable path reached prepare(): {:?}", path); + path_to_db_string(Path::new(&path)) + }); + debug_assert_eq!(db_path, path_to_db_string(Path::new(&db_path))); - let Ok(meta) = crate::platform::metadata_or_stat(&path, cached) else { + let Ok(meta) = crate::platform::metadata_or_stat(Path::new(&db_path), cached) else { // Seen but unreadable: a transient stat failure must not read as // "deleted". return WalkedFile::skipped(db_path, aliased); @@ -373,17 +380,7 @@ fn prepare( return WalkedFile::skipped(db_path, aliased); }; - let action = match known { - Known::InDir(rows) => { - // `to_str`, never lossy; the screen makes it always `Some`. - let name = path - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or_default(); - classify_for_indexing(name, mtime, rows) - } - Known::Exact(stored) => classify_by_mtime(stored, mtime), - }; + let action = classify_by_mtime(stored, mtime); let record = match action { // Unchanged: never opened or hashed, so it stays at one syscall. FileIndexAction::Skip => None, @@ -414,20 +411,24 @@ fn worker(shared: &Shared, ctx: &Ctx, tx: &mpsc::SyncSender) { let delivered = match job { Job::Dir(dir, rows) => { let (mut found, mut stale) = (Vec::new(), Vec::new()); - let files = read_directory(&dir, &rows, ctx, &mut found, &mut stale); + let files = read_directory(&dir, rows, ctx, &mut found, &mut stale); // Hand the subdirectories over before doing our own per-file // work, so the rest of the pool never idles behind one worker. slot.finish(found); - deliver(files, &rows, stale, ctx, tx, &mut scratch) + deliver(files, stale, ctx, tx, &mut scratch) } - Job::Files(files, rows) => { + Job::Files(files) => { slot.finish(Vec::new()); - deliver(files, &rows, Vec::new(), ctx, tx, &mut scratch) + deliver(files, Vec::new(), ctx, tx, &mut scratch) } Job::Alias(path, stored) => { slot.finish(Vec::new()); - let file = PendingFile { path, cached: None }; - let walked = prepare(file, Known::Exact(stored), ctx, &mut scratch); + let file = PendingFile { + path, + cached: None, + stored, + }; + let walked = prepare(file, true, ctx, &mut scratch); tx.send(WalkEvent::File(walked)).is_ok() } }; @@ -443,7 +444,6 @@ fn worker(shared: &Shared, ctx: &Ctx, tx: &mpsc::SyncSender) { /// failed: not an error. fn deliver( files: Vec, - rows: &DirRows, stale: Vec, ctx: &Ctx, tx: &mpsc::SyncSender, @@ -456,7 +456,7 @@ fn deliver( if ctx.stop_flag.load(Ordering::Relaxed) { return false; } - let walked = prepare(file, Known::InDir(rows), ctx, scratch); + let walked = prepare(file, false, ctx, scratch); tx.send(WalkEvent::File(walked)).is_ok() }) } @@ -478,7 +478,7 @@ fn prefetcher(shared: &Shared, db_path: &str) { while let Some(work) = shared.take_prefetch() { let job = match work { PrefetchWork::Dir(dir) => crate::db::repo::dir_rows(&conn, &dir_to_db_parent(&dir)) - .map(|rows| Job::Dir(dir, Arc::new(rows))), + .map(|rows| Job::Dir(dir, rows)), PrefetchWork::Alias(path) => { crate::db::repo::mtime_for_path(&conn, &path_to_db_string(&path)) .map(|stored| Job::Alias(path, stored)) diff --git a/crates/quicksearch-core/src/walk/pool.rs b/crates/quicksearch-core/src/walk/pool.rs index 17e3e99..b430492 100644 --- a/crates/quicksearch-core/src/walk/pool.rs +++ b/crates/quicksearch-core/src/walk/pool.rs @@ -17,7 +17,8 @@ pub(super) struct Queue { /// end-of-walk proof in [`Shared::take`]. pub(super) prefetching: bool, /// What [`PREFETCH_AHEAD`] bounds. Not `jobs.len()`: that also counts - /// `Job::Files` chunks, which carry no rows and would starve prefetching. + /// `Job::Files` chunks, which hold no directory's rows and would starve + /// prefetching. pub(super) dirs_ready: usize, pub(super) active: usize, /// Canonical directories already queued: collapses overlapping roots, @@ -173,8 +174,8 @@ pub(super) enum PrefetchWork { pub(super) enum Found { Dir(PathBuf), Alias(PathBuf), - /// Overflow files from the directory just read, which already has rows. - Files(Vec, Arc), + /// Overflow files from the directory just read, each carrying its row. + Files(Vec), } impl Shared { @@ -192,7 +193,7 @@ impl Shared { } } Found::Alias(path) => q.needs_alias.push(path), - Found::Files(files, rows) => q.jobs.push(Job::Files(files, rows)), + Found::Files(files) => q.jobs.push(Job::Files(files)), } } q.active -= 1; diff --git a/crates/quicksearch-core/src/walk/tests.rs b/crates/quicksearch-core/src/walk/tests.rs index 14674d5..a5f20da 100644 --- a/crates/quicksearch-core/src/walk/tests.rs +++ b/crates/quicksearch-core/src/walk/tests.rs @@ -177,9 +177,9 @@ fn a_non_utf8_name_is_dropped_at_the_entry() { ); } -/// A `continue` in `read_directory` leaves the name out of `present`, and -/// everything left out of `present` is deleted; the row spelled with the lossy -/// name belongs to a different file, which must mark itself present. +/// A `continue` in `read_directory` leaves the name's row in `rows`, and +/// every row left there is deleted; the row spelled with the lossy name +/// belongs to a different file, which must take its own row out. #[test] fn dropping_a_bad_entry_deletes_nothing() { let root = tmp_tree("nonutf8-stale"); @@ -659,6 +659,50 @@ fn a_directory_reports_rows_with_no_file_behind_them() { ); } +/// A directory wider than one job is spread across the pool, and each file +/// takes its stored mtime with it: every chunk must classify against the +/// file's own row, and only the row with no file is stale. +#[test] +fn every_file_of_a_wide_directory_is_classified_against_its_own_row() { + let root = tmp_tree("wide"); + let total = FILES_PER_JOB * 2 + 7; + let path = |i: usize| root.join(format!("f{:04}.txt", i)); + for i in 0..total { + touch(&path(i), b"x"); + } + let mtime = + |i: usize| crate::file_handling::mtime_secs(&fs::metadata(path(i)).unwrap()).unwrap(); + // Thirds: unchanged, changed, never indexed. + let mut rows: Vec<(String, u64)> = (0..total) + .filter(|i| i % 3 != 2) + .map(|i| (path_to_db_string(&path(i)), mtime(i) + (i % 3) as u64)) + .collect(); + let gone = path_to_db_string(&root.join("gone.txt")); + rows.push((gone.clone(), 1)); + let db = db_with("wide", &rows); + + let (mut files, mut stale) = (Vec::new(), Vec::new()); + for event in start(&root, &db) { + match event { + WalkEvent::File(f) => files.push(f), + WalkEvent::Stale(paths) => stale.extend(paths), + } + } + + assert_eq!(files.len(), total); + for f in &files { + let name = Path::new(&f.path).file_name().unwrap().to_str().unwrap(); + let i: usize = name[1..5].parse().unwrap(); + let want = [ + FileIndexAction::Skip, + FileIndexAction::Update, + FileIndexAction::Insert, + ][i % 3]; + assert_eq!(f.action, want, "{}", name); + } + assert_eq!(stale, vec![gone], "only the row with no file behind it"); +} + #[test] #[cfg(unix)] fn an_unreadable_directory_reports_nothing_stale() { @@ -819,7 +863,7 @@ fn the_index_and_its_sidecars_are_never_walked() { /// A stored row for the index itself has to go, or the live watcher and /// duplicate verification keep opening the file; a skip would keep the row, -/// so the walk leaves the name out of the directory's `present` set instead. +/// so the walk leaves the name's row in the directory's `rows` instead. #[test] fn a_stored_row_for_the_index_falls_to_the_stale_sweep() { let root = crate::testutil::scratch_dir_canonical("walk-self-index-stale"); diff --git a/crates/quicksearch-core/src/watcher.rs b/crates/quicksearch-core/src/watcher.rs index 7cced54..52ac986 100644 --- a/crates/quicksearch-core/src/watcher.rs +++ b/crates/quicksearch-core/src/watcher.rs @@ -6,12 +6,12 @@ //! the whole watcher fails: a half-watched root looks live while going //! silently stale. -use std::collections::{BTreeSet, HashMap}; +use std::collections::BTreeSet; use std::path::{Path, PathBuf}; -use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; +use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::{mpsc, Arc, Mutex}; use std::thread::{self, JoinHandle}; -use std::time::{Duration, Instant}; +use std::time::Duration; use notify::event::{ModifyKind, RenameMode}; use notify::{ @@ -116,20 +116,15 @@ fn fmt_cap(cap: usize) -> String { } } -/// Tuning for the whole filesystem-event pipeline. +/// Tuning for the whole filesystem-event pipeline. The watcher forwards +/// events as they arrive; the coordinator's queue coalesces them by path and +/// decides when to apply them. #[derive(Debug, Clone)] pub struct WatcherConfig { /// How long the coordinator's queue must go quiet before it is applied. pub pending_settle: Duration, /// Ceiling on how long `pending_settle` may hold the queue back. pub pending_max_defer: Duration, - /// Per-directory debounce window. - pub throttle_window: Duration, - pub tick_interval: Duration, - /// Directories *selected* per tick; [`FLUSH_BUDGET`] is the real limit. - pub max_dirs_per_tick: usize, - /// When to garbage-collect stale throttle entries (idle > window * N). - pub prune_max_age_multiplier: u32, /// Directory budget; see [`DEFAULT_MAX_WATCHED_DIRS`]. pub max_watched_dirs: usize, } @@ -139,10 +134,6 @@ impl Default for WatcherConfig { Self { pending_settle: Duration::from_secs(2), pending_max_defer: Duration::from_secs(30), - throttle_window: Duration::from_secs(30), - tick_interval: Duration::from_millis(500), - max_dirs_per_tick: 512, - prune_max_age_multiplier: 10, max_watched_dirs: DEFAULT_MAX_WATCHED_DIRS, } } @@ -156,8 +147,6 @@ struct WatchRegistry { cap: usize, } -/// How long one flush pass may spend handing events to the sink. -const FLUSH_BUDGET: Duration = Duration::from_millis(50); const WATCH_MODE: RecursiveMode = if crate::platform::WATCH_ROOTS_RECURSIVELY { RecursiveMode::Recursive @@ -240,7 +229,9 @@ fn add_watch(reg: &mut WatchRegistry, dir: &Path) -> Result<(), WatchError> { /// Handle to a running watcher. Dropping calls [`Self::stop`] implicitly. pub struct Watcher { - stop_flag: Arc, + /// The loop's own channel: `None` ends it. The notify handle holds the + /// other sender, so the channel never closes while the loop runs. + stop_tx: mpsc::Sender>, handle: Option>, dir_count: Arc, degraded: Degraded, @@ -261,7 +252,8 @@ impl Watcher { config: WatcherConfig, sink: EventSink, ) -> Result { - let (tx, rx) = mpsc::channel::(); + let (tx, rx) = mpsc::channel::>(); + let stop_tx = tx.clone(); let degraded: Degraded = Arc::new(Mutex::new(None)); let degraded_cb = degraded.clone(); let raw = RecommendedWatcher::new( @@ -274,7 +266,7 @@ impl Watcher { crate::lock_ok(°raded_cb).get_or_insert(WatchError::Overflowed); return; } - let _ = tx.send(ev); + let _ = tx.send(Some(ev)); } Err(e) => { // The limit can also be hit asynchronously, when notify @@ -330,11 +322,8 @@ impl Watcher { } let dir_count = Arc::new(AtomicUsize::new(crate::lock_ok(®istry).dirs.len())); - let stop_flag = Arc::new(AtomicBool::new(false)); let ctx = LoopCtx { sink, - config, - stop: stop_flag.clone(), registry: registry.clone(), filters, roots, @@ -344,7 +333,7 @@ impl Watcher { let handle = thread::spawn(move || run_loop(rx, ctx)); Ok(Self { - stop_flag, + stop_tx, handle: Some(handle), dir_count, degraded, @@ -376,7 +365,7 @@ impl Watcher { /// Stop the background thread and join it; safe to call multiple times. pub fn stop(&mut self) { - self.stop_flag.store(true, Ordering::Relaxed); + let _ = self.stop_tx.send(None); if let Some(h) = self.handle.take() { let _ = h.join(); } @@ -400,8 +389,6 @@ impl std::fmt::Debug for Watcher { struct LoopCtx { sink: EventSink, - config: WatcherConfig, - stop: Arc, registry: Arc>, filters: WatchFilters, /// Configured roots; "hidden" is judged relative to them, since a root @@ -412,56 +399,21 @@ struct LoopCtx { } #[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum QueuedOp { +enum Op { Create, Modify, Remove, } -#[derive(Debug)] -struct DirThrottleEntry { - record_time: Instant, - queue: HashMap, - /// Flush on the next tick regardless of window age; set for the first - /// event in a previously-idle directory so it reacts fast. - immediate: bool, -} - -/// Ticks between sweeps of idle throttle entries. -const PRUNE_EVERY_TICKS: u32 = 20; - -fn run_loop(rx: mpsc::Receiver, ctx: LoopCtx) { - let mut throttle: HashMap = HashMap::new(); - let mut tick_counter: u32 = 0; - - while !ctx.stop.load(Ordering::Relaxed) { - let deadline = Instant::now() + ctx.config.tick_interval; - while !ctx.stop.load(Ordering::Relaxed) { - let remaining = deadline.saturating_duration_since(Instant::now()); - if remaining.is_zero() { - break; - } - match rx.recv_timeout(remaining) { - Ok(ev) => handle_notify_event(&ev, &mut throttle, &ctx), - Err(mpsc::RecvTimeoutError::Timeout) => break, - Err(mpsc::RecvTimeoutError::Disconnected) => return, - } - } - - if ctx.stop.load(Ordering::Relaxed) { - break; - } - - flush_ready(&mut throttle, &ctx.sink, &ctx.config); - - tick_counter = tick_counter.wrapping_add(1); - if tick_counter.is_multiple_of(PRUNE_EVERY_TICKS) { - let max_age = ctx - .config - .throttle_window - .saturating_mul(ctx.config.prune_max_age_multiplier); - prune_stale(&mut throttle, max_age); - } +/// Forward each interesting event to the sink as it arrives, blocking in +/// between; `None` is [`Watcher::stop`]. Nothing is held back here: the +/// coordinator's queue coalesces by path and waits for `pending_settle` +/// (at most `pending_max_defer`). A per-directory throttle here used to hold +/// a directory's second event for 30 s and woke this thread twice a second +/// while idle. +fn run_loop(rx: mpsc::Receiver>, ctx: LoopCtx) { + while let Ok(Some(ev)) = rx.recv() { + handle_notify_event(&ev, &ctx); } } @@ -525,7 +477,7 @@ fn unwatch_tree(ctx: &LoopCtx, path: &Path) { /// Whether an event for `path` is worth queueing at all: the same predicate /// the walk applies. Under a recursive root watch it alone keeps -/// `node_modules` churn out of the throttle map. A path the index cannot +/// `node_modules` churn out of the coordinator's queue. A path the index cannot /// spell is dropped (see `path_to_db_string`). fn is_event_interesting(ctx: &LoopCtx, path: &Path) -> bool { path.to_str().is_some() @@ -533,11 +485,7 @@ fn is_event_interesting(ctx: &LoopCtx, path: &Path) -> bool { && (ctx.filters.include_hidden || !path_has_hidden_component_under(path, &ctx.roots)) } -fn handle_notify_event( - ev: &NotifyEvent, - throttle: &mut HashMap, - ctx: &LoopCtx, -) { +fn handle_notify_event(ev: &NotifyEvent, ctx: &LoopCtx) { let op = match ev.kind { EventKind::Modify(ModifyKind::Name(RenameMode::Both)) if ev.paths.len() == 2 => { // A rename is only uninteresting when *both* ends are: out of an @@ -556,13 +504,9 @@ fn handle_notify_event( } // Split renames degrade to Create/Remove; notify 6.x does not bundle // halves by cookie uniformly across backends. - EventKind::Create(_) | EventKind::Modify(ModifyKind::Name(RenameMode::To)) => { - QueuedOp::Create - } - EventKind::Remove(_) | EventKind::Modify(ModifyKind::Name(RenameMode::From)) => { - QueuedOp::Remove - } - EventKind::Modify(_) => QueuedOp::Modify, + EventKind::Create(_) | EventKind::Modify(ModifyKind::Name(RenameMode::To)) => Op::Create, + EventKind::Remove(_) | EventKind::Modify(ModifyKind::Name(RenameMode::From)) => Op::Remove, + EventKind::Modify(_) => Op::Modify, _ => return, }; @@ -570,83 +514,20 @@ fn handle_notify_event( if !is_event_interesting(ctx, p) { continue; } - // Keep the watch set in step before debouncing: a directory created - // now may be populated before its window expires and needs its watch. - match op { - QueuedOp::Create => watch_if_new_dir(ctx, p), - QueuedOp::Remove => unwatch_tree(ctx, p), - QueuedOp::Modify => {} - } - enqueue(throttle, p.clone(), op); - } -} - -fn enqueue(throttle: &mut HashMap, path: PathBuf, op: QueuedOp) { - let dir = path - .parent() - .map(|p| p.to_path_buf()) - .unwrap_or_else(|| path.clone()); - let entry = throttle.entry(dir).or_insert_with(|| DirThrottleEntry { - record_time: Instant::now(), - queue: HashMap::new(), - immediate: true, - }); - // Created and deleted within one window: the file never surfaces. - if op == QueuedOp::Remove && entry.queue.get(&path) == Some(&QueuedOp::Create) { - entry.queue.remove(&path); - } else { - entry.queue.insert(path, op); - } -} - -fn flush_ready( - throttle: &mut HashMap, - sink: &EventSink, - config: &WatcherConfig, -) { - let now = Instant::now(); - let mut ready: Vec = Vec::new(); - for (dir, entry) in throttle.iter() { - let age = now.saturating_duration_since(entry.record_time); - if entry.immediate || (!entry.queue.is_empty() && age >= config.throttle_window) { - ready.push(dir.clone()); - if ready.len() >= config.max_dirs_per_tick { - break; + // Keep the watch set in step before the event is applied: a + // directory created now may be populated at once and needs its watch. + let event = match op { + Op::Create => { + watch_if_new_dir(ctx, p); + FsEvent::Create(p.clone()) } - } - } - let deadline = now + FLUSH_BUDGET; - for dir in ready { - if Instant::now() >= deadline { - break; - } - if let Some(entry) = throttle.get_mut(&dir) { - let drained: Vec<(PathBuf, QueuedOp)> = entry.queue.drain().collect(); - entry.immediate = false; - entry.record_time = now; - for (path, op) in drained { - let ev = match op { - QueuedOp::Create => FsEvent::Create(path), - QueuedOp::Modify => FsEvent::Modify(path), - QueuedOp::Remove => FsEvent::Remove(path), - }; - sink(ev); + Op::Remove => { + unwatch_tree(ctx, p); + FsEvent::Remove(p.clone()) } - } - } -} - -fn prune_stale(throttle: &mut HashMap, max_age: Duration) { - let now = Instant::now(); - throttle.retain(|_, entry| { - !entry.queue.is_empty() - || entry.immediate - || now.saturating_duration_since(entry.record_time) < max_age - }); - // `retain` keeps the table sized for the busiest moment this map has ever - // seen; a single unpack burst would otherwise hold that memory for days. - if throttle.is_empty() { - throttle.shrink_to_fit(); + Op::Modify => FsEvent::Modify(p.clone()), + }; + (ctx.sink)(event); } } diff --git a/crates/quicksearch-core/src/watcher_tests.rs b/crates/quicksearch-core/src/watcher_tests.rs index 1cabf64..15d8c71 100644 --- a/crates/quicksearch-core/src/watcher_tests.rs +++ b/crates/quicksearch-core/src/watcher_tests.rs @@ -18,19 +18,11 @@ fn default_filters() -> WatchFilters { } } -fn fast_config() -> WatcherConfig { - WatcherConfig { - throttle_window: Duration::from_millis(50), - tick_interval: Duration::from_millis(20), - ..WatcherConfig::default() - } -} - -/// [`fast_config`] with a directory budget of `n`. +/// The default config with a directory budget of `n`. fn capped(n: usize) -> WatcherConfig { WatcherConfig { max_watched_dirs: n, - ..fast_config() + ..WatcherConfig::default() } } @@ -39,9 +31,9 @@ fn watch_with(dir: &Path, config: WatcherConfig, sink: EventSink) -> Result Watcher { - watch_with(dir, fast_config(), sink_to_vec().0).unwrap() + watch_with(dir, WatcherConfig::default(), sink_to_vec().0).unwrap() } /// Seeds `dirs` directly so set logic is tested without real kernel watches. @@ -72,88 +64,44 @@ fn remove_tree_skips_the_scan_for_a_path_that_is_not_watched() { ); } +/// Nothing holds an event back: a directory's second change reaches the +/// sink as promptly as its first. A per-directory throttle once held it for +/// 30 s; the coordinator's queue does the debouncing now. #[test] -fn enqueue_create_then_remove_cancels() { - let mut map: HashMap = HashMap::new(); - let p = PathBuf::from("/tmp/a.txt"); - enqueue(&mut map, p.clone(), QueuedOp::Create); - enqueue(&mut map, p.clone(), QueuedOp::Remove); - let entry = map.get(p.parent().unwrap()).unwrap(); - assert!(entry.queue.is_empty(), "Create then Remove should cancel"); -} - -#[test] -fn modify_after_modify_is_one() { - let mut map: HashMap = HashMap::new(); - let p = PathBuf::from("/tmp/a.txt"); - enqueue(&mut map, p.clone(), QueuedOp::Modify); - enqueue(&mut map, p.clone(), QueuedOp::Modify); - let entry = map.get(p.parent().unwrap()).unwrap(); - assert_eq!(entry.queue.len(), 1); -} - -#[test] -fn flush_ready_leading_edge_fires_immediately() { - let mut map: HashMap = HashMap::new(); - enqueue(&mut map, PathBuf::from("/tmp/a.txt"), QueuedOp::Create); +fn every_event_reaches_the_sink_as_it_happens() { + let dir = crate::testutil::scratch_dir("prompt"); let (sink, got) = sink_to_vec(); - let config = WatcherConfig::default(); - flush_ready(&mut map, &sink, &config); - let got = got.lock().unwrap(); - assert_eq!(got.len(), 1); - assert!(matches!(got[0], FsEvent::Create(_))); -} + let mut w = watch_with(&dir, WatcherConfig::default(), sink).unwrap(); -#[test] -fn flush_ready_respects_max_dirs_per_tick() { - let mut map: HashMap = HashMap::new(); - for i in 0..10 { - enqueue( - &mut map, - PathBuf::from(format!("/dir{}/a", i)), - QueuedOp::Create, - ); - } - let (sink, got) = sink_to_vec(); - let config = WatcherConfig { - max_dirs_per_tick: 3, - ..WatcherConfig::default() + let seen = |name: &str| { + let path = dir.join(name); + let deadline = std::time::Instant::now() + Duration::from_secs(5); + while std::time::Instant::now() < deadline { + let found = got + .lock() + .unwrap() + .iter() + .any(|e| matches!(e, FsEvent::Create(p) | FsEvent::Modify(p) if p == &path)); + if found { + return true; + } + std::thread::sleep(Duration::from_millis(10)); + } + false }; - flush_ready(&mut map, &sink, &config); - // Each dir contributes one event because each entry has one path. - assert_eq!(got.lock().unwrap().len(), 3); -} - -#[test] -fn prune_stale_drops_empty_old_entries() { - let mut map: HashMap = HashMap::new(); - map.insert( - PathBuf::from("/tmp"), - DirThrottleEntry { - record_time: Instant::now() - Duration::from_secs(3600), - queue: HashMap::new(), - immediate: false, - }, + std::fs::write(dir.join("first.txt"), "a").unwrap(); + assert!(seen("first.txt")); + let second = std::time::Instant::now(); + std::fs::write(dir.join("second.txt"), "b").unwrap(); + assert!(seen("second.txt")); + assert!( + second.elapsed() < Duration::from_secs(2), + "the second event in a directory took {:?}", + second.elapsed() ); - prune_stale(&mut map, Duration::from_secs(1)); - assert!(map.is_empty()); -} -#[test] -fn prune_stale_keeps_active_entries() { - let mut map: HashMap = HashMap::new(); - let mut queue = HashMap::new(); - queue.insert(PathBuf::from("/tmp/a"), QueuedOp::Modify); - map.insert( - PathBuf::from("/tmp"), - DirThrottleEntry { - record_time: Instant::now() - Duration::from_secs(3600), - queue, - immediate: false, - }, - ); - prune_stale(&mut map, Duration::from_secs(1)); - assert_eq!(map.len(), 1); + w.stop(); + std::fs::remove_dir_all(&dir).ok(); } #[test] @@ -161,7 +109,7 @@ fn e2e_create_modify_remove_surfaces() { let dir = crate::testutil::scratch_dir("e2e"); let (sink, got) = sink_to_vec(); - let mut w = watch_with(&dir, fast_config(), sink).unwrap(); + let mut w = watch_with(&dir, WatcherConfig::default(), sink).unwrap(); let f = dir.join("hello.txt"); std::fs::write(&f, "hi").unwrap(); @@ -200,7 +148,7 @@ fn events_for_an_unrepresentable_name_never_surface() { std::fs::remove_file(&bad).unwrap(); let (sink, got) = sink_to_vec(); - let mut w = watch_with(&dir, fast_config(), sink).unwrap(); + let mut w = watch_with(&dir, WatcherConfig::default(), sink).unwrap(); std::fs::write(&bad, "hi").unwrap(); std::thread::sleep(Duration::from_millis(150)); @@ -270,7 +218,13 @@ fn include_hidden_registers_dotted_dirs() { include_hidden: true, ..default_filters() }; - let w = Watcher::start(vec![dir.clone()], filters, fast_config(), sink_to_vec().0).unwrap(); + let w = Watcher::start( + vec![dir.clone()], + filters, + WatcherConfig::default(), + sink_to_vec().0, + ) + .unwrap(); assert_eq!(w.watched_dirs(), 2, "root + .hidden"); drop(w); @@ -303,7 +257,7 @@ fn an_unreadable_directory_is_skipped_not_fatal() { std::fs::create_dir_all(&locked).unwrap(); crate::platform::deny_read(&locked).unwrap(); - let started = watch_with(&dir, fast_config(), sink_to_vec().0); + let started = watch_with(&dir, WatcherConfig::default(), sink_to_vec().0); crate::platform::restore_read(&locked).ok(); let w = started.expect("an unreadable directory must not fail the watcher"); @@ -351,7 +305,7 @@ fn a_tree_inside_the_cap_registers() { fn a_directory_created_after_start_is_watched() { let dir = crate::testutil::scratch_dir("newdir"); let (sink, got) = sink_to_vec(); - let mut w = watch_with(&dir, fast_config(), sink).unwrap(); + let mut w = watch_with(&dir, WatcherConfig::default(), sink).unwrap(); assert_eq!(w.watched_dirs(), 1, "only the root to begin with"); let sub = dir.join("later"); diff --git a/crates/quicksearch-core/tests/common/mod.rs b/crates/quicksearch-core/tests/common/mod.rs index a9b9862..d4cc999 100644 --- a/crates/quicksearch-core/tests/common/mod.rs +++ b/crates/quicksearch-core/tests/common/mod.rs @@ -14,10 +14,13 @@ pub use quicksearch_core::testutil::{touch, Lcg, BODY_TERM}; const INDEX_TIMEOUT: Duration = Duration::from_secs(120); -/// One full indexing run, awaited via the `last_full_index` marker, which is -/// written only on a successful finish. Polling for `Idle` would race: a small -/// tree finishes between two polls, leaving `Idle` ambiguous between "not -/// started yet" and "already done". +/// One full indexing run, awaited until `Idle`, then checked for the +/// `last_full_index` marker, which is written only on a successful finish. +/// +/// `Idle` and not the marker ends the wait: the marker lands before the +/// run's tail and the post-run VACUUM, which would otherwise still be +/// writing when the caller's next statement runs. `Idle` is unambiguous +/// because `start_indexing` claims the status before it returns. pub struct IndexOnce<'a> { pub db: &'a Path, pub roots: Vec, @@ -25,8 +28,8 @@ pub struct IndexOnce<'a> { /// Delete any existing completion marker first. Off for suites indexing a /// database whose lifecycle they are themselves testing. pub fresh_marker: bool, - /// Poll the marker through the keyed open: a plain open cannot read an - /// encrypted index and would never observe its own completion. + /// Read the marker through the keyed open: a plain open cannot read an + /// encrypted index. pub encrypted: bool, } @@ -48,28 +51,29 @@ impl IndexOnce<'_> { .unwrap(); let deadline = Instant::now() + INDEX_TIMEOUT; - let mut done = false; - while Instant::now() < deadline { - if let IndexingStatus::Error(e) = service.get_status() { - panic!("indexing failed: {}", e); - } - if self.db.exists() && self.completed() { - done = true; - break; + loop { + match service.get_status() { + IndexingStatus::Idle => break, + IndexingStatus::Error(e) => panic!("indexing failed: {}", e), + _ => {} } + assert!( + Instant::now() < deadline, + "indexing did not finish within {:?}", + INDEX_TIMEOUT + ); std::thread::sleep(Duration::from_millis(10)); } - assert!(done, "indexing did not finish within {:?}", INDEX_TIMEOUT); - service.stop_indexing().unwrap(); + assert!(self.completed(), "the run ended without finishing"); } - /// Whether the marker is present; mid-creation is simply "not yet". + /// Whether the marker is present. fn completed(&self) -> bool { let conn = if self.encrypted { - db::open_existing(&self.db.to_string_lossy(), false).ok() + db::open_existing(&self.db.to_string_lossy(), false).unwrap() } else { - rusqlite::Connection::open(self.db).ok() + rusqlite::Connection::open(self.db).unwrap() }; - conn.is_some_and(|c| db::repo::get_last_full_index(&c).is_some()) + db::repo::get_last_full_index(&conn).is_some() } } diff --git a/crates/quicksearch-gui/src/app/status_bar.rs b/crates/quicksearch-gui/src/app/status_bar.rs index 1279f85..8ef2fd5 100644 --- a/crates/quicksearch-gui/src/app/status_bar.rs +++ b/crates/quicksearch-gui/src/app/status_bar.rs @@ -146,11 +146,6 @@ impl QuickSearchApp { { ctx.request_repaint_after(Duration::from_millis(250)); } - // Watcher registration walks every root, so its verdict can land - // minutes after startup, with no input to repaint on. - if matches!(state.watcher, WatcherStatus::Starting) { - ctx.request_repaint_after(Duration::from_millis(500)); - } } } diff --git a/crates/quicksearch-gui/src/hotkey/mod.rs b/crates/quicksearch-gui/src/hotkey/mod.rs index d1b494d..8545900 100644 --- a/crates/quicksearch-gui/src/hotkey/mod.rs +++ b/crates/quicksearch-gui/src/hotkey/mod.rs @@ -52,13 +52,16 @@ struct Registry { } enum Backend { - /// Nothing registered: no shortcut set, a Wayland session (see the - /// module docs), or the backend never started. + /// A Wayland session (see the module docs): nothing is ever registered. Idle, - /// Windows and X11. + /// Windows and X11, holding no key, and so no manager either: on X11 one + /// polls the display twenty times a second for as long as it exists, + /// whether or not it holds anything. + Ready, + /// Windows and X11, holding `registered`. Grab { manager: GlobalHotKeyManager, - registered: Option, + registered: HotKey, }, } @@ -75,34 +78,27 @@ pub fn init(ctx: &egui::Context, setting: &str) { } })); - let (registry, started) = match choose_backend() { - Ok(backend) => ( - Registry { - backend, - status: Status::Disabled, - }, - true, - ), - Err(message) => { - quicksearch_core::log_warn!("global shortcut: {}", message); - ( - Registry { - backend: Backend::Idle, - status: Status::Error(message), - }, - false, - ) - } + // No X11 grab as a fallback under Wayland: it succeeds inside a Wayland + // session and then only fires while an XWayland window has focus, which + // looks broken. + let backend = if crate::activate::raise::is_wayland() { + Backend::Idle + } else { + Backend::Ready }; - REGISTRY.with_borrow_mut(|slot| *slot = Some(registry)); - if started { - apply(setting); - } + REGISTRY.with_borrow_mut(|slot| { + *slot = Some(Registry { + backend, + status: Status::Disabled, + }) + }); + apply(setting); } /// Registers `setting`, releasing whatever was registered before. An /// unparseable or refused shortcut is reported through [`status`], never by -/// failing: not worth blocking a config the user has already applied. +/// failing: not worth blocking a config the user has already applied. On the +/// event-loop thread, like [`init`]: registering creates the manager. pub fn apply(setting: &str) { REGISTRY.with_borrow_mut(|slot| { let Some(registry) = slot.as_mut() else { @@ -156,53 +152,43 @@ pub fn status() -> Status { } impl Backend { - /// Hold `wanted` and nothing else. `None` releases without registering. + /// Hold `wanted` and nothing else. `None` releases without registering, + /// and drops the manager with it. fn register(&mut self, wanted: Option) -> Result<(), String> { - match self { - Backend::Idle => Ok(()), - Backend::Grab { - manager, - registered, - } => { - if let Some(old) = registered.take() { - // Worth reporting, not worth refusing the new binding over. - if let Err(e) = manager.unregister(old) { - quicksearch_core::log_warn!("releasing the old global shortcut: {}", e); - } - } - let Some(binding) = wanted else { - return Ok(()); - }; - let hotkey: HotKey = binding - .to_string() - .parse() - .map_err(|e| format!("{} is not a usable shortcut: {}", binding, e))?; - manager.register(hotkey).map_err(|e| match e { - global_hotkey::Error::AlreadyRegistered(_) => { - format!("another application is already using {}", binding) - } - other => format!("{} could not be registered: {}", binding, other), - })?; - *registered = Some(hotkey); - Ok(()) + if matches!(self, Backend::Idle) { + return Ok(()); + } + if let Backend::Grab { + manager, + registered, + } = std::mem::replace(self, Backend::Ready) + { + // Worth reporting, not worth refusing the new binding over. + if let Err(e) = manager.unregister(registered) { + quicksearch_core::log_warn!("releasing the old global shortcut: {}", e); } } - } -} - -/// A Wayland session gets no backend (see the module docs). No X11 grab as a -/// fallback either: it succeeds inside a Wayland session and then only fires -/// while an XWayland window has focus, which looks broken. -fn choose_backend() -> Result { - if crate::activate::raise::is_wayland() { - return Ok(Backend::Idle); - } - GlobalHotKeyManager::new() - .map(|manager| Backend::Grab { + let Some(binding) = wanted else { + return Ok(()); + }; + let hotkey: HotKey = binding + .to_string() + .parse() + .map_err(|e| format!("{} is not a usable shortcut: {}", binding, e))?; + let manager = GlobalHotKeyManager::new() + .map_err(|e| format!("global shortcuts are unavailable: {}", e))?; + manager.register(hotkey).map_err(|e| match e { + global_hotkey::Error::AlreadyRegistered(_) => { + format!("another application is already using {}", binding) + } + other => format!("{} could not be registered: {}", binding, other), + })?; + *self = Backend::Grab { manager, - registered: None, - }) - .map_err(|e| format!("global shortcuts are unavailable: {}", e)) + registered: hotkey, + }; + Ok(()) + } } #[cfg(test)] @@ -226,5 +212,15 @@ mod tests { backend.register(Some("Ctrl+Shift+F".parse().unwrap())), Ok(()) ); + assert!(matches!(backend, Backend::Idle)); + } + + /// Holding no key must not start a manager: on X11 its thread polls the + /// display until it is dropped. + #[test] + fn releasing_holds_no_manager() { + let mut backend = Backend::Ready; + assert_eq!(backend.register(None), Ok(())); + assert!(matches!(backend, Backend::Ready)); } } diff --git a/crates/quicksearch-gui/src/search_tab.rs b/crates/quicksearch-gui/src/search_tab.rs index 1ae423b..2620b92 100644 --- a/crates/quicksearch-gui/src/search_tab.rs +++ b/crates/quicksearch-gui/src/search_tab.rs @@ -568,7 +568,6 @@ impl SearchTab { return; } match update { - SearchUpdate::Started { .. } => {} SearchUpdate::Hits { hits, .. } => { // An empty batch is not a result; the cascade never sends one, // but the event is public and this is what it would mean. diff --git a/crates/quicksearch-gui/src/search_tab/tests.rs b/crates/quicksearch-gui/src/search_tab/tests.rs index fa588cd..48e32a9 100644 --- a/crates/quicksearch-gui/src/search_tab/tests.rs +++ b/crates/quicksearch-gui/src/search_tab/tests.rs @@ -53,7 +53,11 @@ fn timed_frame(ctx: &egui::Context, tab: &mut SearchTab) { /// Per-visible-row frame cost, printed rather than asserted. Measured: 1000 /// rows with content snippets cost 615 µs/frame (<4% of a 60 fps budget), so /// the Content Match cut is not memoized; re-measure here before optimizing -/// the row renderer. +/// the row renderer. Re-measured for formatting each hit's size, date, rank +/// and flattened window once on arrival instead of every frame: rows cost +/// 231 µs/frame (names only) and 434 µs (content snippets), against about +/// 20 ms for a whole frame under software GL (llvmpipe), so caching could +/// save at most ~2% of a fade or cursor-blink frame. Not adopted. /// /// ```text /// QSB_RENDER_PERF=1 cargo test --release -p quicksearch-gui -- render_perf --nocapture