From a8dc067cd4339e826af154bd29a74667049ad37f Mon Sep 17 00:00:00 2001 From: dailz Date: Sun, 7 Jun 2026 08:31:00 +0800 Subject: [PATCH] fix(bench): [M23] prevent single_frame_tail/head overlap for small files - Extract shared FRAME_LINES constant into suites/mod.rs - Add select_frame_positions() helper with 3*FRAME_LINES threshold to guarantee non-overlapping head/middle/tail ranges - Guard bench_reverse_scan against total <= FRAME_LINES - Add 9 boundary tests for position selection (0..1M lines) Closes #42 --- crates/bench/src/suites/jump.rs | 15 ++- crates/bench/src/suites/mod.rs | 5 + crates/bench/src/suites/render.rs | 156 ++++++++++++++++++++++++++++-- 3 files changed, 162 insertions(+), 14 deletions(-) diff --git a/crates/bench/src/suites/jump.rs b/crates/bench/src/suites/jump.rs index b49b695..d0c9f6f 100644 --- a/crates/bench/src/suites/jump.rs +++ b/crates/bench/src/suites/jump.rs @@ -1,5 +1,6 @@ use std::collections::HashMap; +use super::FRAME_LINES; use crate::metrics::MetricsCollector; use crate::mmap_reader::{ MmapReaderPhaseAware, MmapReaderPlain, MmapReaderPopulate, MmapReaderRandom, @@ -210,13 +211,17 @@ fn bench_reverse_scan( let path = &config.test_file; let reader = B::open(path).expect("Failed to open file"); let total = reader.total_lines(); - let iterations: usize = if config.quick_mode { 5 } else { 10 }; - let mut latencies = Vec::with_capacity(iterations * 35); + if total <= FRAME_LINES { + reader.close(); + return vec![]; + } + + let iterations: usize = if config.quick_mode { 5 } else { 10 }; + let mut latencies = Vec::with_capacity(iterations * FRAME_LINES); for _ in 0..iterations { - // Read lines backwards from end: last line, last-1, ..., last-34 - let start = total.saturating_sub(35); + let start = total - FRAME_LINES; for i in (start..total).rev() { let t = std::time::Instant::now(); let _ = reader.get_line(i); @@ -229,7 +234,7 @@ fn bench_reverse_scan( reader.close(); let mut extra = HashMap::new(); - extra.insert("lines_per_scan".into(), 35.0); + extra.insert("lines_per_scan".into(), FRAME_LINES as f64); extra.insert("iterations".into(), iterations as f64); vec![BenchmarkResult { diff --git a/crates/bench/src/suites/mod.rs b/crates/bench/src/suites/mod.rs index 7c72fea..7f51d45 100644 --- a/crates/bench/src/suites/mod.rs +++ b/crates/bench/src/suites/mod.rs @@ -5,3 +5,8 @@ pub mod memory; pub mod render; pub mod rotation; pub mod startup; + +/// Number of lines read per single-frame render benchmark. +/// Also used as the scan window size for reverse-scan benchmarks. +/// Shared across suites to ensure consistent workload sizing. +pub(crate) const FRAME_LINES: usize = 35; diff --git a/crates/bench/src/suites/render.rs b/crates/bench/src/suites/render.rs index 59147de..cd033d1 100644 --- a/crates/bench/src/suites/render.rs +++ b/crates/bench/src/suites/render.rs @@ -1,5 +1,6 @@ use std::collections::HashMap; +use super::FRAME_LINES; use crate::metrics::MetricsCollector; use crate::mmap_reader::{ MmapReaderPhaseAware, MmapReaderPlain, MmapReaderPopulate, MmapReaderRandom, @@ -88,15 +89,9 @@ fn bench_single_frame( let total = reader.total_lines(); let mut results = Vec::new(); - let positions = [ - ("head", 0), - ("middle", total / 2), - ("tail", total.saturating_sub(35)), - ]; - - for (pos_name, start_line) in positions { - let mut latencies = Vec::with_capacity(35); - for i in 0..35 { + for (pos_name, start_line) in select_frame_positions(total) { + let mut latencies = Vec::with_capacity(FRAME_LINES); + for i in 0..FRAME_LINES { let line_idx = start_line + i; if line_idx >= total { break; @@ -126,6 +121,33 @@ fn bench_single_frame( results } +/// Select non-overlapping benchmark positions for single-frame tests. +/// +/// Returns `(label, start_line)` pairs. Positions are chosen so that no +/// two frames overlap, skipping any position that cannot fit without +/// colliding with a previously selected range. +pub(crate) fn select_frame_positions(total: usize) -> Vec<(&'static str, usize)> { + let frame = FRAME_LINES; + if total == 0 { + return vec![]; + } + + let mut positions: Vec<(&'static str, usize)> = vec![("head", 0)]; + + // Need at least 3 full frames to place head, middle, and tail without overlap. + if total >= 3 * frame { + let head_end = frame; + let tail_start = total - frame; + let gap = tail_start - head_end; + // Center the middle frame within the gap, ensuring it fits entirely + let middle_start = head_end + gap.saturating_sub(frame) / 2; + positions.push(("middle", middle_start)); + positions.push(("tail", tail_start)); + } + + positions +} + fn bench_continuous_scroll( backend: &str, variant: &str, @@ -175,3 +197,119 @@ fn bench_continuous_scroll( extra, }] } + +#[cfg(test)] +mod tests { + use super::*; + + fn starts(pos: &[(&str, usize)]) -> Vec { + pos.iter().map(|&(_, s)| s).collect() + } + + fn labels<'a>(pos: &[(&'a str, usize)]) -> Vec<&'a str> { + pos.iter().map(|&(l, _)| l).collect() + } + + fn ranges_overlap(a_start: usize, b_start: usize) -> bool { + let a_end = a_start + FRAME_LINES; + let b_end = b_start + FRAME_LINES; + a_start < b_end && b_start < a_end + } + + #[test] + fn zero_lines_no_positions() { + assert!(select_frame_positions(0).is_empty()); + } + + #[test] + fn one_line_head_only() { + let pos = select_frame_positions(1); + assert_eq!(labels(&pos), vec!["head"]); + assert_eq!(starts(&pos), vec![0]); + } + + #[test] + fn at_frame_lines_head_only() { + let pos = select_frame_positions(FRAME_LINES); + assert_eq!(labels(&pos), vec!["head"]); + assert_eq!(starts(&pos), vec![0]); + } + + #[test] + fn just_above_threshold_head_middle_tail() { + let total = 3 * FRAME_LINES; + let pos = select_frame_positions(total); + assert_eq!(labels(&pos), vec!["head", "middle", "tail"]); + assert_eq!(starts(&pos)[0], 0); + assert_eq!(*starts(&pos).last().unwrap(), total - FRAME_LINES); + // Verify no overlap + for i in 0..pos.len() { + for j in (i + 1)..pos.len() { + assert!( + !ranges_overlap(pos[i].1, pos[j].1), + "overlap: {:?} @ {} vs {:?} @ {} (total={})", + pos[i].0, pos[i].1, pos[j].0, pos[j].1, total + ); + } + } + } + + #[test] + fn no_overlap_at_total_70() { + // total=70 < 3*35=105, so only head is returned + let pos = select_frame_positions(70); + assert_eq!(labels(&pos), vec!["head"]); + } + + #[test] + fn no_overlap_at_total_104() { + let pos = select_frame_positions(104); + for i in 0..pos.len() { + for j in (i + 1)..pos.len() { + assert!( + !ranges_overlap(pos[i].1, pos[j].1), + "overlap at total=104: {:?} @ {} vs {:?} @ {}", + pos[i].0, pos[i].1, pos[j].0, pos[j].1 + ); + } + } + } + + #[test] + fn no_overlap_at_total_105() { + let pos = select_frame_positions(105); + for i in 0..pos.len() { + for j in (i + 1)..pos.len() { + assert!( + !ranges_overlap(pos[i].1, pos[j].1), + "overlap at total=105: {:?} @ {} vs {:?} @ {}", + pos[i].0, pos[i].1, pos[j].0, pos[j].1 + ); + } + } + } + + #[test] + fn middle_is_centered_between_head_and_tail() { + let total = 1000; + let pos = select_frame_positions(total); + assert_eq!(pos.len(), 3); + + let (_, head_start) = pos[0]; + let (_, middle_start) = pos[1]; + let (_, tail_start) = pos[2]; + + assert_eq!(head_start, 0); + assert_eq!(tail_start, total - FRAME_LINES); + + let head_end = head_start + FRAME_LINES; + let gap = tail_start - head_end; + assert_eq!(middle_start, head_end + gap.saturating_sub(FRAME_LINES) / 2); + } + + #[test] + fn large_file_all_three_positions() { + let pos = select_frame_positions(1_000_000); + assert_eq!(labels(&pos), vec!["head", "middle", "tail"]); + } +}