diff --git a/.alfonso/release-notes/next/todo-scan-bounds.md b/.alfonso/release-notes/next/todo-scan-bounds.md new file mode 100644 index 0000000000..93a475bc78 --- /dev/null +++ b/.alfonso/release-notes/next/todo-scan-bounds.md @@ -0,0 +1 @@ +Background TODO scans no longer read plain JSON, and TODO, metrics, complexity and duplicate scans bound source reads and parsing while reporting oversized files as coverage gaps. diff --git a/benchmarks/aft-search/slice-descriptors/land-todo-scan-bounds.json b/benchmarks/aft-search/slice-descriptors/land-todo-scan-bounds.json new file mode 100644 index 0000000000..27e9e44660 --- /dev/null +++ b/benchmarks/aft-search/slice-descriptors/land-todo-scan-bounds.json @@ -0,0 +1,8 @@ +{ + "fixtures": [ + "harness-goldens" + ], + "kind": "harness", + "slice_class": "non_ranking", + "targeted_mechanism": "none" +} diff --git a/crates/aft/src/inspect/cache.rs b/crates/aft/src/inspect/cache.rs index fb71ba5b08..3f99e91f25 100644 --- a/crates/aft/src/inspect/cache.rs +++ b/crates/aft/src/inspect/cache.rs @@ -200,7 +200,9 @@ impl From for InspectCacheError { /// v42: corrected source-buffer freshness, impl scopes, complexity and cycles; /// an atomic epoch marker fences raw facts as well as derived findings, and /// undecodable dead-code rows remain explicit coverage gaps. -pub(crate) const TIER2_CONTRIBUTION_CACHE_VERSION: u32 = 42; +/// v43: source-size admission excludes oversized complexity/duplicate inputs +/// and reports their coverage gaps instead of reusing unbounded scan results. +pub(crate) const TIER2_CONTRIBUTION_CACHE_VERSION: u32 = 43; // Reserved metadata category: its integer is an epoch, never a scan timestamp. const CACHE_VERSION_CATEGORY: &str = "__cache_version__"; @@ -3996,7 +3998,7 @@ pub(crate) mod tests { decoded.contribution["exports"][0]["is_type_like"].as_bool(), Some(true) ); - assert_eq!(TIER2_CONTRIBUTION_CACHE_VERSION, 42); + assert_eq!(TIER2_CONTRIBUTION_CACHE_VERSION, 43); } #[test] diff --git a/crates/aft/src/inspect/scanners/complexity.rs b/crates/aft/src/inspect/scanners/complexity.rs index 83672d03d9..2a63e53ac0 100644 --- a/crates/aft/src/inspect/scanners/complexity.rs +++ b/crates/aft/src/inspect/scanners/complexity.rs @@ -8,7 +8,9 @@ use serde_json::{json, Value}; use tree_sitter::Node; use super::source_text::{read_source_text, write_skipped_files, SourceText, NOT_VALID_UTF8}; -use crate::cache_freshness::{self, FileFreshness}; +#[cfg(test)] +use crate::cache_freshness; +use crate::cache_freshness::FileFreshness; use crate::inspect::{ FileContribution, InspectCategory, InspectJob, InspectResult, InspectScanSuccess, }; @@ -347,7 +349,11 @@ pub fn run_complexity_scan(job: &InspectJob) -> InspectResult { }; let aggregate = aggregate_file_scans(&job.project_root, &scans, Some(DRILL_DOWN_LIMIT)); - let scanned_files = scans.iter().map(|scan| scan.path.clone()).collect(); + let scanned_files = scans + .iter() + .filter(|scan| scan.contribution.skipped_reason.is_none()) + .map(|scan| scan.path.clone()) + .collect(); let contributions = scans .iter() .map(file_scan_to_contribution) @@ -381,7 +387,7 @@ pub(crate) fn aggregate_complexity_contributions_with_limit( /// and this scan and is treated as deleted. A file that is not valid UTF-8 is /// a per-file skip; other read failures keep failing the scan as before. fn scan_file(project_root: &Path, path: &Path) -> Result, String> { - let freshness = match cache_freshness::collect(path) { + let (freshness, _permit) = match super::source_text::source_freshness(path) { Ok(freshness) => freshness, Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None), Err(error) => return Err(format!("freshness failed for {}: {error}", path.display())), @@ -391,10 +397,15 @@ fn scan_file(project_root: &Path, path: &Path) -> Result super::source_text::MAX_SOURCE_BYTES { + unreadable_contribution(project_root, path, super::source_text::TOO_LARGE) + } else if analyzable { match read_source_text(path) { Ok(SourceText::Text(source)) => scan_source(project_root, path, &source), Ok(SourceText::NotUtf8) => unreadable_contribution(project_root, path, NOT_VALID_UTF8), + Ok(SourceText::TooLarge) => { + unreadable_contribution(project_root, path, super::source_text::TOO_LARGE) + } Ok(SourceText::Vanished) => return Ok(None), Err(error) => return Err(format!("read failed for {}: {error}", path.display())), } @@ -668,7 +679,7 @@ fn aggregate_contributions( "worst": worst, "items": items, "drill_down_capped": count > limit, - "scanned_files": contributions.len(), + "scanned_files": contributions.iter().filter(|contribution| contribution.skipped_reason.is_none()).count(), "languages_skipped": languages_skipped.into_iter().collect::>(), "complete": parse_errors.is_empty(), }); diff --git a/crates/aft/src/inspect/scanners/duplicates.rs b/crates/aft/src/inspect/scanners/duplicates.rs index a5edef0e82..c6fbdb9f04 100644 --- a/crates/aft/src/inspect/scanners/duplicates.rs +++ b/crates/aft/src/inspect/scanners/duplicates.rs @@ -214,6 +214,7 @@ pub fn run_duplicates_scan(job: &InspectJob) -> InspectResult { .collect::>(); let scanned_files = file_scans .iter() + .filter(|scan| scan.skipped_reason.is_none()) .map(|scan| scan.path.clone()) .collect::>(); @@ -230,11 +231,24 @@ pub fn run_duplicates_scan(job: &InspectJob) -> InspectResult { /// a per-file skip; other read failures keep failing the scan as before. fn scan_file(job: &InspectJob, path: &Path) -> Result, String> { let display_path = display_path(&job.project_root, path); - let freshness = match cache_freshness::collect(path) { + let (freshness, _permit) = match super::source_text::source_freshness(path) { Ok(freshness) => freshness, Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None), Err(error) => return Err(format!("freshness failed for {}: {error}", path.display())), }; + if freshness.size > super::source_text::MAX_SOURCE_BYTES { + return Ok(Some(FileScan { + path: path.to_path_buf(), + display_path, + language_skipped: None, + skipped_reason: Some(super::source_text::TOO_LARGE), + freshness, + line_count: 0, + expected_duplicate: false, + generated: false, + fragments: Vec::new(), + })); + } let Some(lang) = detect_language(path) else { return Ok(Some(FileScan { path: path.to_path_buf(), @@ -265,16 +279,24 @@ fn scan_file(job: &InspectJob, path: &Path) -> Result, String> let source = match read_source_text(path) { Ok(SourceText::Text(source)) => source, - Ok(SourceText::NotUtf8) => { + Ok(skipped @ (SourceText::NotUtf8 | SourceText::TooLarge)) => { return Ok(Some(FileScan { path: path.to_path_buf(), display_path, language_skipped: None, - skipped_reason: Some(NOT_VALID_UTF8), + skipped_reason: Some(if matches!(skipped, SourceText::TooLarge) { + super::source_text::TOO_LARGE + } else { + NOT_VALID_UTF8 + }), freshness, line_count: 0, expected_duplicate: false, - generated: crate::inspect::generated::is_generated_file(&job.project_root, path), + generated: if matches!(skipped, SourceText::TooLarge) { + false + } else { + crate::inspect::generated::is_generated_file(&job.project_root, path) + }, fragments: Vec::new(), })); } @@ -631,7 +653,10 @@ pub(crate) fn aggregate_duplicate_contributions_with_limit( .collect(); let mut aggregate = aggregate_duplicate_occurrences( by_hash, - parsed.len(), + parsed + .iter() + .filter(|scan| scan.skipped_reason.is_none()) + .count(), line_counts.values().copied().map(u64::from).sum(), marker_files, languages_skipped, @@ -796,7 +821,10 @@ fn aggregate_file_scans_with_limit( aggregate_duplicate_occurrences( by_hash, - file_scans.len(), + file_scans + .iter() + .filter(|scan| scan.skipped_reason.is_none()) + .count(), file_scans .iter() .map(|scan| u64::from(scan.line_count)) diff --git a/crates/aft/src/inspect/scanners/metrics.rs b/crates/aft/src/inspect/scanners/metrics.rs index 46c37e3e99..d81ef0931b 100644 --- a/crates/aft/src/inspect/scanners/metrics.rs +++ b/crates/aft/src/inspect/scanners/metrics.rs @@ -31,6 +31,7 @@ struct CachedFileMetric { path: PathBuf, language: &'static str, loc: usize, + skipped_reason: Option<&'static str>, } #[derive(Debug, Clone)] @@ -90,18 +91,10 @@ fn run_metrics_scan_with_memo( let started = Instant::now(); memo.reserve_for_scan(job.scope_files.len()); let progress = crate::run_tool_call::progress::FileProgress::current(); - let per_file = job + let cached_files = job .scope_files .par_iter() - .map(|path| { - let cached = memo.get_or_insert_with(path, scan_file); - FileMetric { - path: cached.path, - language: cached.language, - symbol_count: cached_symbol_count(path, job), - loc: cached.loc, - } - }) + .map(|path| memo.get_or_insert_with(path, scan_file)) .inspect(|_| { if let Some(progress) = &progress { progress.finished(); @@ -111,9 +104,29 @@ fn run_metrics_scan_with_memo( if job.is_full_project_scope() { memo.prune_to_scope(&job.project_root, &job.scope_files); } - let aggregate = aggregate_metrics(&job.project_root, &per_file); + let mut skipped_files = Vec::new(); + let per_file = cached_files + .into_iter() + .filter_map(|cached| { + if let Some(reason) = cached.skipped_reason { + skipped_files.push(( + display_path(&job.project_root, &cached.path), + reason.to_string(), + )); + return None; + } + Some(FileMetric { + symbol_count: cached_symbol_count(&cached.path, job), + path: cached.path, + language: cached.language, + loc: cached.loc, + }) + }) + .collect::>(); + let mut aggregate = aggregate_metrics(&job.project_root, &per_file); + super::source_text::write_skipped_files(&mut aggregate, skipped_files, None); let success = crate::inspect::InspectScanSuccess { - scanned_files: job.scope_files.clone(), + scanned_files: per_file.iter().map(|file| file.path.clone()).collect(), contributions: Vec::new(), aggregate, }; @@ -127,9 +140,12 @@ fn metrics_memo() -> &'static Tier1FileMemo { fn scan_file(path: &Path) -> (Option, CachedFileMetric) { let metadata = fs::metadata(path).ok(); - let bytes = read_file_bytes(path); + let _permit = + super::source_text::source_scan_permit(metadata.as_ref().map_or(0, fs::Metadata::len)); + let (bytes, skipped_reason) = read_file_bytes(path); let freshness = metadata .as_ref() + .filter(|_| skipped_reason.is_none()) .map(|metadata| freshness_from_metadata(metadata, bytes.as_deref())); let loc = bytes.as_deref().map(line_count_bytes).unwrap_or_default(); @@ -137,6 +153,7 @@ fn scan_file(path: &Path) -> (Option, CachedFileMetric) { path: path.to_path_buf(), language: language_key(path), loc, + skipped_reason, }; (freshness, metric) } @@ -158,10 +175,17 @@ fn freshness_from_metadata(metadata: &fs::Metadata, bytes: Option<&[u8]>) -> Fil } } -fn read_file_bytes(path: &Path) -> Option> { - #[cfg(debug_assertions)] - bump_file_read_count(path); - crate::project_content::read(path).ok() +fn read_file_bytes(path: &Path) -> (Option>, Option<&'static str>) { + use super::source_text::{read_source_bytes, SourceBytes, TOO_LARGE}; + match read_source_bytes(path) { + Ok(SourceBytes::Bytes(bytes)) => { + #[cfg(debug_assertions)] + bump_file_read_count(path); + (Some(bytes), None) + } + Ok(SourceBytes::TooLarge) => (None, Some(TOO_LARGE)), + _ => (None, None), + } } fn cached_symbol_count(path: &Path, job: &crate::inspect::InspectJob) -> usize { diff --git a/crates/aft/src/inspect/scanners/source_text.rs b/crates/aft/src/inspect/scanners/source_text.rs index 39ee90c0d9..6e8883b062 100644 --- a/crates/aft/src/inspect/scanners/source_text.rs +++ b/crates/aft/src/inspect/scanners/source_text.rs @@ -10,15 +10,146 @@ //! as the file's cached contribution, so it is not re-read until the file //! changes, and a changed file that is now valid UTF-8 is analyzed normally. +use std::io::Read; use std::path::Path; #[cfg(debug_assertions)] use std::path::PathBuf; +use std::sync::{Condvar, Mutex}; use serde_json::{json, Value}; /// Reason recorded for a file whose bytes are not valid UTF-8. pub(crate) const NOT_VALID_UTF8: &str = "not valid UTF-8"; +pub(crate) const MAX_SOURCE_BYTES: u64 = crate::search_index::DEFAULT_MAX_FILE_SIZE; +// Agent-visible reason, approved by Ufuk; change it only with his approval. +pub(crate) const TOO_LARGE: &str = "over the 1 MiB size limit"; + +// Charge source bytes, not file count, so small files can use the whole Rayon +// pool. Reservations stay live through parsing and tree traversal. Source-byte +// limits bound input, not the amplified size of parser trees and scratch space. +pub(super) const SOURCE_SCAN_BUDGET_BYTES: u64 = 32 * 1024 * 1024; +static SOURCE_SCANS: SourceScanBudget = SourceScanBudget::new(SOURCE_SCAN_BUDGET_BYTES); + +struct SourceScanBudget { + in_flight: Mutex, + available: Condvar, + capacity: u64, +} + +impl SourceScanBudget { + const fn new(capacity: u64) -> Self { + Self { + in_flight: Mutex::new(0), + available: Condvar::new(), + capacity, + } + } + + fn reserve(&self, stat_size: u64) -> SourceScanPermit<'_> { + let bytes = stat_size.min(MAX_SOURCE_BYTES); + let mut active = self.in_flight.lock().unwrap_or_else(|e| e.into_inner()); + // One file must make progress even if the budget is smaller than that + // file's reservation; other nonempty reservations wait for its release. + while *active != 0 && bytes > self.capacity.saturating_sub(*active) { + active = self + .available + .wait(active) + .unwrap_or_else(|e| e.into_inner()); + } + *active += bytes; + SourceScanPermit { + budget: self, + bytes, + } + } +} + +pub(crate) struct SourceScanPermit<'a> { + budget: &'a SourceScanBudget, + bytes: u64, +} + +pub(crate) fn source_scan_permit(stat_size: u64) -> SourceScanPermit<'static> { + SOURCE_SCANS.reserve(stat_size) +} + +impl Drop for SourceScanPermit<'_> { + fn drop(&mut self) { + let mut active = self + .budget + .in_flight + .lock() + .unwrap_or_else(|e| e.into_inner()); + *active -= self.bytes; + self.budget.available.notify_all(); + } +} + +pub(crate) enum SourceBytes { + Bytes(Vec), + TooLarge, + Vanished, +} + +/// Admit by metadata before opening, then bound the read too: a file can grow +/// between stat and read. The extra byte detects overflow, never a partial parse. +pub(crate) fn read_source_bytes(path: &Path) -> std::io::Result { + if crate::project_content::is_excluded(path) { + return Ok(SourceBytes::Vanished); + } + let read = || -> std::io::Result { + let metadata = std::fs::metadata(path)?; + if metadata.len() > MAX_SOURCE_BYTES { + return Ok(SourceBytes::TooLarge); + } + let file = crate::project_content::open(path)?; + if file.metadata()?.len() > MAX_SOURCE_BYTES { + return Ok(SourceBytes::TooLarge); + } + #[cfg(debug_assertions)] + bump_source_read_count(path); + let mut bytes = Vec::new(); + file.take(MAX_SOURCE_BYTES + 1).read_to_end(&mut bytes)?; + crate::views::io::WORK + .content_read_bytes + .fetch_add(bytes.len() as u64, std::sync::atomic::Ordering::Relaxed); + Ok(if bytes.len() as u64 > MAX_SOURCE_BYTES { + SourceBytes::TooLarge + } else { + SourceBytes::Bytes(bytes) + }) + }; + match read() { + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(SourceBytes::Vanished), + result => result, + } +} + +/// Oversized contributions need metadata for invalidation, not a content hash. +/// Keep the returned reservation through the caller's parsing and traversal; +/// it also covers any source read needed to compute freshness. +pub(crate) fn source_freshness( + path: &Path, +) -> std::io::Result<( + crate::cache_freshness::FileFreshness, + SourceScanPermit<'static>, +)> { + let metadata = std::fs::metadata(path)?; + let permit = source_scan_permit(metadata.len()); + if metadata.len() > MAX_SOURCE_BYTES { + return Ok(( + crate::cache_freshness::FileFreshness { + mtime: metadata.modified().unwrap_or(std::time::UNIX_EPOCH), + size: metadata.len(), + content_hash: crate::cache_freshness::zero_hash(), + }, + permit, + )); + } + crate::cache_freshness::collect(path).map(|freshness| (freshness, permit)) +} + /// Most skipped files named in a project-wide aggregate. The total is always /// reported in `skipped_files_count`, so a capped list is never mistaken for /// the whole set. @@ -31,6 +162,8 @@ pub(crate) enum SourceText { Text(String), /// The file was read but is not valid UTF-8; the scanner skips it. NotUtf8, + /// The file exceeds the source limit and was not parsed. + TooLarge, /// The file disappeared or is excluded from project source (for example, /// internal storage). No contribution, exactly as the next walk will see it. Vanished, @@ -40,18 +173,13 @@ pub(crate) enum SourceText { /// example permission denied) are returned unchanged so each scanner keeps /// its existing contract for them. pub(crate) fn read_source_text(path: &Path) -> std::io::Result { - if crate::project_content::is_excluded(path) { - return Ok(SourceText::Vanished); - } - #[cfg(debug_assertions)] - bump_source_read_count(path); - match crate::project_content::read(path) { - Ok(bytes) => Ok(match String::from_utf8(bytes) { + match read_source_bytes(path)? { + SourceBytes::Bytes(bytes) => Ok(match String::from_utf8(bytes) { Ok(text) => SourceText::Text(text), Err(_) => SourceText::NotUtf8, }), - Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(SourceText::Vanished), - Err(error) => Err(error), + SourceBytes::TooLarge => Ok(SourceText::TooLarge), + SourceBytes::Vanished => Ok(SourceText::Vanished), } } @@ -117,6 +245,188 @@ pub fn source_read_count_for_debug(path: &Path) -> usize { mod tests { use super::*; + #[test] + fn source_budget_charges_bytes_and_small_files_can_pass_a_waiting_large_file() { + let budget = std::sync::Arc::new(SourceScanBudget::new(MAX_SOURCE_BYTES)); + let large = budget.reserve(3 * MAX_SOURCE_BYTES / 4); + let (entered_tx, entered_rx) = crossbeam_channel::bounded(1); + let (release_tx, release_rx) = crossbeam_channel::bounded(1); + let worker_budget = budget.clone(); + let worker = std::thread::spawn(move || { + let _permit = worker_budget.reserve(MAX_SOURCE_BYTES / 2); + entered_tx.send(()).unwrap(); + release_rx.recv().unwrap(); + }); + let small = budget.reserve(MAX_SOURCE_BYTES / 4); + assert_eq!(*budget.in_flight.lock().unwrap(), MAX_SOURCE_BYTES); + let blocked = entered_rx + .recv_timeout(std::time::Duration::from_millis(100)) + .is_err(); + drop(small); + drop(large); + entered_rx + .recv_timeout(std::time::Duration::from_secs(5)) + .unwrap(); + assert_eq!(*budget.in_flight.lock().unwrap(), MAX_SOURCE_BYTES / 2); + release_tx.send(()).unwrap(); + worker.join().unwrap(); + assert!( + blocked, + "a reservation exceeded the shared source-byte budget" + ); + assert_eq!(*budget.in_flight.lock().unwrap(), 0); + } + + #[test] + fn a_single_file_is_admitted_even_when_its_clamped_reservation_exceeds_the_budget() { + let budget = std::sync::Arc::new(SourceScanBudget::new(MAX_SOURCE_BYTES / 2)); + let worker_budget = budget.clone(); + let (entered_tx, entered_rx) = crossbeam_channel::bounded(1); + let (release_tx, release_rx) = crossbeam_channel::bounded(1); + let worker = std::thread::spawn(move || { + let permit = worker_budget.reserve(4 * MAX_SOURCE_BYTES); + entered_tx.send(permit.bytes).unwrap(); + release_rx.recv().unwrap(); + }); + assert_eq!( + entered_rx + .recv_timeout(std::time::Duration::from_secs(5)) + .expect("single file must not deadlock"), + MAX_SOURCE_BYTES + ); + assert_eq!(*budget.in_flight.lock().unwrap(), MAX_SOURCE_BYTES); + release_tx.send(()).unwrap(); + worker.join().unwrap(); + assert_eq!(*budget.in_flight.lock().unwrap(), 0); + } + + /// Measure native parser allocations in an isolated Linux test process; + /// Rust-only allocation counters do not include tree-sitter's C allocations. + #[cfg(target_os = "linux")] + #[test] + #[ignore = "native parse-memory probe; requires AFT_TREE_MEMORY_LANGUAGE on Linux"] + fn source_tree_memory_probe() { + let language = std::env::var("AFT_TREE_MEMORY_LANGUAGE").unwrap(); + let (language_id, source) = match language.as_str() { + "json" => ( + crate::parser::LangId::Json, + format!( + "[{}0]", + "{\"k\":1},".repeat(MAX_SOURCE_BYTES as usize / 8 - 1) + ), + ), + "rust" => { + let mut source = String::new(); + let mut index = 0; + while source.len() < MAX_SOURCE_BYTES as usize - 100 { + source.push_str(&format!( + "fn f{index}(x:i32)->i32{{if x>0{{x+1}}else{{x-1}}}}\n" + )); + index += 1; + } + (crate::parser::LangId::Rust, source) + } + _ => panic!("expected json or rust"), + }; + let mut parser = tree_sitter::Parser::new(); + parser + .set_language(&crate::parser::grammar_for(language_id)) + .unwrap(); + let in_use = || { + crate::memory::AllocatorObservation::measure() + .snapshot + .bytes_in_use + .expect("native allocator census") + }; + let before = in_use(); + let tree = parser.parse(&source, None).unwrap(); + assert!(!tree.root_node().has_error()); + let live = in_use(); + std::hint::black_box(&tree); + drop(tree); + let after_tree_drop = in_use(); + eprintln!("TREE_MEMORY language={language} source_bytes={} parse_live_bytes={} tree_owned_bytes={} parser_retained_bytes={} live_multiple={:.2}", source.len(), live.saturating_sub(before), live.saturating_sub(after_tree_drop), after_tree_drop.saturating_sub(before), live.saturating_sub(before) as f64 / source.len() as f64); + } + + #[test] + fn source_size_limit_is_pinned_and_enforced_before_open() { + assert_eq!(MAX_SOURCE_BYTES, 1_048_576); + assert_eq!(TOO_LARGE, "over the 1 MiB size limit"); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("source.rs"); + let file = std::fs::File::create(&path).unwrap(); + file.set_len(MAX_SOURCE_BYTES + 1).unwrap(); + let probe = crate::project_content::Probe::new(dir.path()); + assert!(matches!( + read_source_text(&path).unwrap(), + SourceText::TooLarge + )); + assert!(probe.paths().is_empty()); + file.set_len(MAX_SOURCE_BYTES).unwrap(); + assert!( + matches!(read_source_bytes(&path).unwrap(), SourceBytes::Bytes(bytes) if bytes.len() as u64 == MAX_SOURCE_BYTES) + ); + assert_eq!(probe.paths(), vec![path]); + } + + #[test] + fn oversized_sources_are_unopened_named_gaps_in_other_scanners() { + use crate::inspect::{InspectCategory, InspectJob, JobKey, JobScope}; + use std::sync::{Arc, RwLock}; + let dir = tempfile::tempdir().unwrap(); + let large = dir.path().join("large.rs"); + std::fs::File::create(&large) + .unwrap() + .set_len(MAX_SOURCE_BYTES + 1) + .unwrap(); + let small = dir.path().join("small.rs"); + std::fs::write(&small, "fn small() {}\n").unwrap(); + let probe = crate::project_content::Probe::new(dir.path()); + for category in [ + InspectCategory::Metrics, + InspectCategory::Complexity, + InspectCategory::Duplicates, + ] { + let scope = JobScope::for_project(dir.path()); + let job = InspectJob { + job_id: 1, + key: JobKey::for_category_scope(category, &scope), + category, + scope_files: vec![large.clone(), small.clone()], + project_root: dir.path().to_path_buf(), + inspect_dir: dir.path().join("inspect"), + config: Arc::new(crate::config::Config::default()), + symbol_cache: Arc::new(RwLock::new(crate::parser::SymbolCache::new())), + inspect_writer: true, + callgraph_writer: true, + callgraph_snapshot: None, + }; + let result = match category { + InspectCategory::Metrics => super::super::metrics::run_metrics_scan(&job), + InspectCategory::Complexity => super::super::complexity::run_complexity_scan(&job), + InspectCategory::Duplicates => super::super::duplicates::run_duplicates_scan(&job), + _ => unreachable!(), + } + .outcome + .unwrap(); + assert_eq!(result.scanned_files, vec![small.clone()], "{category:?}"); + assert_eq!(result.aggregate["complete"], false, "{category:?}"); + assert_eq!( + result.aggregate["skipped_files"], + json!([ + {"file": "large.rs", "reason": "over the 1 MiB size limit"} + ]), + "{category:?}" + ); + if category == InspectCategory::Metrics { + assert_eq!(result.aggregate["files"], 1); + } else { + assert_eq!(result.aggregate["scanned_files"], 1); + } + } + assert!(!probe.paths().contains(&large)); + } + #[test] fn invalid_utf8_is_a_skip_and_a_missing_file_is_vanished() { let dir = tempfile::tempdir().expect("tempdir"); diff --git a/crates/aft/src/inspect/scanners/todos.rs b/crates/aft/src/inspect/scanners/todos.rs index 853c98ea66..e1d442a40d 100644 --- a/crates/aft/src/inspect/scanners/todos.rs +++ b/crates/aft/src/inspect/scanners/todos.rs @@ -26,6 +26,55 @@ thread_local! { static TODOS_PARSERS: RefCell> = RefCell::new(HashMap::new()); } +/// Isolated cold-scan timing: inventory and fixture preparation stay outside +/// the measured interval, and each invocation uses a fresh test process. +#[cfg(all(test, target_os = "linux"))] +mod throughput_probe { + use super::*; + + #[test] + #[ignore = "repository throughput probe; requires AFT_SCAN_BENCH_ROOT on Linux"] + fn todos_and_metrics_throughput_probe() { + use crate::inspect::{InspectCategory, JobKey, JobScope}; + use std::sync::{Arc, RwLock}; + let root = PathBuf::from(std::env::var_os("AFT_SCAN_BENCH_ROOT").expect("corpus root")); + let mut files = crate::callgraph::walk_project_files(&root).collect::>(); + files.sort(); + let input_bytes = files + .iter() + .map(|path| std::fs::metadata(path).unwrap().len()) + .sum::(); + eprintln!("SCAN_CORPUS files={} bytes={input_bytes}", files.len()); + let scope = JobScope::for_project(&root); + let mut job = InspectJob { + job_id: 1, + key: JobKey::for_category_scope(InspectCategory::Todos, &scope), + category: InspectCategory::Todos, + scope_files: files, + project_root: root.clone(), + inspect_dir: root.join("inspect"), + config: Arc::new(crate::config::Config::default()), + symbol_cache: Arc::new(RwLock::new(crate::parser::SymbolCache::new())), + inspect_writer: true, + callgraph_writer: true, + callgraph_snapshot: None, + }; + let started = Instant::now(); + let todos = run_todos_scan(&job).outcome.expect("TODO scan"); + let todo_ms = started.elapsed().as_secs_f64() * 1000.0; + job.category = InspectCategory::Metrics; + job.key = JobKey::for_category_scope(job.category, &scope); + let started = Instant::now(); + let metrics = super::super::metrics::run_metrics_scan(&job) + .outcome + .expect("metrics scan"); + let metrics_ms = started.elapsed().as_secs_f64() * 1000.0; + eprintln!("SCAN_TIMING files={} todo_ms={todo_ms:.3} metrics_ms={metrics_ms:.3} total_ms={:.3} todo_count={} todo_scanned={} metrics_scanned={}", job.scope_files.len(), todo_ms + metrics_ms, todos.aggregate["count"], todos.scanned_files.len(), metrics.scanned_files.len()); + assert!(!todos.scanned_files.is_empty()); + assert!(!metrics.scanned_files.is_empty()); + } +} + #[cfg(debug_assertions)] static FILE_READS: super::read_counts::FileReadCounts = super::read_counts::FileReadCounts::new(); @@ -41,6 +90,7 @@ struct TodoItem { #[derive(Debug, Clone)] struct FileScan { scanned_file: Option, + skipped: Option<(PathBuf, &'static str)>, items: Vec, } @@ -99,9 +149,16 @@ fn run_todos_scan_with_memo(job: &InspectJob, memo: &Tier1FileMemo) -> } let mut scanned_files = Vec::new(); + let mut skipped_files = Vec::new(); let mut all_items = Vec::new(); let mut excluded = ExcludedTestTally::default(); for scan in per_file { + if let Some((path, reason)) = scan.skipped { + skipped_files.push(( + display_file_path(&job.project_root, &path), + reason.to_string(), + )); + } let file = scan .scanned_file .as_ref() @@ -156,6 +213,7 @@ fn run_todos_scan_with_memo(job: &InspectJob, memo: &Tier1FileMemo) -> "drill_down_capped": drill_down_capped, }); excluded.write_into(&mut aggregate); + super::source_text::write_skipped_files(&mut aggregate, skipped_files, None); let success = InspectScanSuccess { scanned_files, contributions: Vec::new(), @@ -169,12 +227,32 @@ fn todos_memo() -> &'static Tier1FileMemo { } fn scan_file(path: &Path, project_root: &Path) -> (Option, FileScan) { - let (freshness, source) = read_text_file(path); + // Plain JSON cannot contain comments. Do not open it, even to sniff for a + // language; JSON with comments must use a comment-capable extension. + if path + .extension() + .and_then(|ext| ext.to_str()) + .is_some_and(|ext| ext.eq_ignore_ascii_case("json")) + { + return ( + None, + FileScan { + scanned_file: None, + skipped: None, + items: Vec::new(), + }, + ); + } + let metadata = std::fs::metadata(path).ok(); + let _permit = + super::source_text::source_scan_permit(metadata.as_ref().map_or(0, std::fs::Metadata::len)); + let (freshness, source, skipped_reason) = read_text_file(path, metadata); let Some(source) = source else { return ( freshness, FileScan { scanned_file: None, + skipped: skipped_reason.map(|reason| (path.to_path_buf(), reason)), items: Vec::new(), }, ); @@ -190,6 +268,7 @@ fn scan_file(path: &Path, project_root: &Path) -> (Option, FileSc freshness, FileScan { scanned_file: Some(path.to_path_buf()), + skipped: None, items, }, ) @@ -520,22 +599,31 @@ fn next_char_len(line: &str, cursor: usize) -> usize { .unwrap_or_default() } -fn read_text_file(path: &Path) -> (Option, Option) { - let metadata = std::fs::metadata(path).ok(); - #[cfg(debug_assertions)] - bump_file_read_count(path); - let bytes = crate::project_content::read(path).ok(); +fn read_text_file( + path: &Path, + metadata: Option, +) -> (Option, Option, Option<&'static str>) { + use super::source_text::{read_source_bytes, SourceBytes, TOO_LARGE}; + let bytes = match read_source_bytes(path) { + Ok(SourceBytes::Bytes(bytes)) => { + #[cfg(debug_assertions)] + bump_file_read_count(path); + Some(bytes) + } + Ok(SourceBytes::TooLarge) => return (None, None, Some(TOO_LARGE)), + _ => None, + }; let freshness = metadata .as_ref() .map(|metadata| freshness_from_metadata(metadata, bytes.as_deref())); let Some(bytes) = bytes else { - return (freshness, None); + return (freshness, None, None); }; if bytes.contains(&0) { - return (freshness, None); + return (freshness, None, None); } - (freshness, String::from_utf8(bytes).ok()) + (freshness, String::from_utf8(bytes).ok(), None) } fn freshness_from_metadata(metadata: &std::fs::Metadata, bytes: Option<&[u8]>) -> FileFreshness { @@ -762,10 +850,10 @@ mod tests { let dir = tempfile::tempdir().unwrap(); let path = dir.path().join("file.rs"); fs::write(&path, "// TODO: marker\n").unwrap(); - let _ = read_text_file(&path); + let _ = read_text_file(&path, fs::metadata(&path).ok()); assert_eq!(file_read_count_for_debug(dir.path()), 0); reset_file_read_count_for_debug(dir.path()); - let _ = read_text_file(&path); + let _ = read_text_file(&path, fs::metadata(&path).ok()); assert_eq!(file_read_count_for_debug(dir.path()), 1); } use crate::config::Config; @@ -811,6 +899,154 @@ mod tests { } } + #[test] + fn oversized_todo_source_is_unopened_and_is_a_named_gap() { + let project = tempfile::tempdir().unwrap(); + let large = project.path().join("large.rs"); + fs::File::create(&large) + .unwrap() + .set_len(super::super::source_text::MAX_SOURCE_BYTES + 1) + .unwrap(); + let small = project.path().join("small.rs"); + fs::write(&small, "// TODO: small source still scanned\n").unwrap(); + let probe = crate::project_content::Probe::new(project.path()); + let job = todos_job(project.path(), vec![large.clone(), small.clone()]); + let memo = Tier1FileMemo::default(); + for _ in 0..2 { + let result = run_todos_scan_with_memo(&job, &memo).outcome.unwrap(); + assert_eq!(result.scanned_files, vec![small.clone()]); + assert_eq!(result.aggregate["count"], 1); + assert_eq!(result.aggregate["complete"], false); + assert_eq!( + result.aggregate["skipped_files"], + serde_json::json!([ + {"file": "large.rs", "reason": "over the 1 MiB size limit"} + ]) + ); + } + assert!(!probe.paths().contains(&large)); + fs::write(&large, "// TODO: now small enough\n").unwrap(); + let result = run_todos_scan_with_memo(&job, &memo).outcome.unwrap(); + assert_eq!(result.aggregate["count"], 2); + assert!(result.aggregate.get("skipped_files").is_none()); + } + + #[test] + fn plain_json_is_unopened_but_jsonc_comments_are_scanned() { + let project = tempfile::tempdir().unwrap(); + let plain = project.path().join("data.json"); + fs::write(&plain, "{\"value\": \"TODO: not a comment\"}").unwrap(); + let commented = project.path().join("config.jsonc"); + fs::write( + &commented, + "{\n// TODO: keep this comment\n\"value\": 1\n}\n", + ) + .unwrap(); + let probe = crate::project_content::Probe::new(project.path()); + let job = todos_job(project.path(), vec![plain.clone(), commented.clone()]); + let memo = Tier1FileMemo::default(); + for _ in 0..2 { + let result = run_todos_scan_with_memo(&job, &memo).outcome.unwrap(); + assert_eq!(result.scanned_files, vec![commented.clone()]); + assert_eq!(result.aggregate["count"], 1); + assert_eq!(result.aggregate["items"][0]["file"], "config.jsonc"); + assert!(result.aggregate.get("skipped_files").is_none()); + assert_ne!(result.aggregate["complete"], false); + } + assert!(!probe.paths().contains(&plain)); + assert!(probe.paths().contains(&commented)); + } + + #[cfg(debug_assertions)] + #[test] + fn several_large_todo_files_hold_source_bytes_through_parse_without_a_two_file_cap() { + let project = tempfile::tempdir().unwrap(); + let (entered_tx, entered_rx) = crossbeam_channel::unbounded(); + let (release_tx, release_rx) = crossbeam_channel::unbounded(); + let fitting_files = (super::super::source_text::SOURCE_SCAN_BUDGET_BYTES + / super::super::source_text::MAX_SOURCE_BYTES) as usize; + let total_files = fitting_files + 1; + let paths = (0..total_files) + .map(|index| { + let path = project.path().join(format!("large-{index}.rs")); + let mut source = String::from("// TODO: bounded parallel parsing\n"); + source.extend(std::iter::repeat_n( + ' ', + super::super::source_text::MAX_SOURCE_BYTES as usize - source.len(), + )); + fs::write(&path, source).unwrap(); + PARSE_DELAYS + .get_or_init(Default::default) + .lock() + .unwrap() + .insert(path.clone(), (entered_tx.clone(), release_rx.clone())); + path + }) + .collect(); + reset_file_read_count_for_debug(project.path()); + let job = todos_job(project.path(), paths); + let worker = std::thread::spawn(move || { + rayon::ThreadPoolBuilder::new() + .num_threads(total_files) + .build() + .unwrap() + .install(|| { + run_todos_scan_with_memo(&job, &Tier1FileMemo::default()) + .outcome + .unwrap() + }) + }); + let deadline = std::time::Duration::from_secs(5); + for _ in 0..fitting_files { + entered_rx.recv_timeout(deadline).unwrap(); + } + let over_budget = entered_rx.recv_timeout(std::time::Duration::from_millis(100)); + let reads_while_blocked = file_read_count_for_debug(project.path()); + for _ in 0..total_files { + release_tx.send(()).unwrap(); + } + let result = worker.join().unwrap(); + assert!( + over_budget.is_err(), + "another parser ran while the source-byte budget was fully reserved" + ); + assert_eq!( + reads_while_blocked, fitting_files, + "permits must cover reads, not only parsing" + ); + assert_eq!(result.scanned_files.len(), total_files); + assert_eq!(result.aggregate["count"], total_files); + } + + /// Run in its own process under `/usr/bin/time -v`; fixtures are prepared + /// separately so their allocation and write cost do not enter the peak + /// resident set size (RSS). + #[cfg(target_os = "linux")] + #[test] + #[ignore = "large-input RSS probe; requires AFT_TODO_RSS_ROOT on Linux"] + fn todos_large_json_rss_probe() { + let root = PathBuf::from(std::env::var_os("AFT_TODO_RSS_ROOT").expect("fixture root")); + let paths = [ + "data-100.json", + "data-125.json", + "data-150.json", + "small.rs", + ] + .map(|name| root.join(name)) + .to_vec(); + reset_file_read_count_for_debug(&root); + let result = run_todos_scan_with_memo(&todos_job(&root, paths), &Tier1FileMemo::default()) + .outcome + .expect("TODO scan"); + assert_eq!(result.aggregate["count"], 1); + eprintln!( + "TODO RSS probe: reads={}, scanned={}, aggregate={}", + file_read_count_for_debug(&root), + result.scanned_files.len(), + result.aggregate + ); + } + #[test] fn quiesced_root_aborts_real_executor_todos_scan_mid_parse() { executor_todos_abandonment(false, false);