diff --git a/README.md b/README.md index 5d9fa40f..f684c500 100644 --- a/README.md +++ b/README.md @@ -267,9 +267,10 @@ The paths to the directories or archives of the recorded data to be included in The directory and archive name of the report. -`-t, --tmp-dir ` [default: /tmp] +`--time-range RUN_NAME=FROM_TIME:TO_TIME` -Temporary directory for intermediate files +The time range to apply to a run in the report, including its time-series metrics, statistics, and analytical findings. +Specify the option multiple times to apply a time range for multiple runs, or omit the `RUN_NAME=` part to apply it to all runs. Either bound can be omitted or negative. ----- diff --git a/src/data/common/processed_data_accessor.rs b/src/data/common/processed_data_accessor.rs index 78a64c4d..cc322a6c 100644 --- a/src/data/common/processed_data_accessor.rs +++ b/src/data/common/processed_data_accessor.rs @@ -13,9 +13,9 @@ use std::fmt::Write; #[derive(Debug)] pub struct ProcessedDataAccessor { /// The start of the time range for every run data - per_run_from_time: HashMap, + per_run_from_time: HashMap, /// The end of the time range for every run data - per_run_to_time: HashMap, + per_run_to_time: HashMap, /// The cache for a metric's stat within the time range /// (since stat computation is costly). time_series_metric_stat_cache: HashMap<(String, String), Statistics>, @@ -31,8 +31,8 @@ impl ProcessedDataAccessor { } pub fn from_time_ranges( - per_run_from_time: HashMap, - per_run_to_time: HashMap, + per_run_from_time: HashMap, + per_run_to_time: HashMap, ) -> Self { ProcessedDataAccessor { per_run_from_time, @@ -267,8 +267,8 @@ fn write_time_series_metric_json_string( buf: &mut String, time_series_metric: &TimeSeriesMetric, stats: Statistics, - from_time: Option, - to_time: Option, + from_time: Option, + to_time: Option, ) { buf.push_str("{\"metric_name\":"); write!( @@ -307,8 +307,8 @@ fn write_time_series_metric_json_string( fn write_series_json_string( buf: &mut String, series: &Series, - from_time: Option, - to_time: Option, + from_time: Option, + to_time: Option, ) { let (start, end) = compute_time_diff_index(&series.time_diff, from_time, to_time); // Uses itoa/ryu for fast int/float number formatting @@ -377,20 +377,38 @@ fn get_key_value_data<'a>( /// the specified time range. fn compute_time_diff_index( time_diff: &[u64], - from_time: Option, - to_time: Option, + from_time: Option, + to_time: Option, ) -> (usize, usize) { let start_idx = if let Some(from) = from_time { - time_diff.partition_point(|t| *t < from) + let from_t = if from < 0 { + time_diff + .last() + .map(|&last_time_diff| last_time_diff.saturating_sub(from.abs() as u64)) + .unwrap_or(0) + } else { + from as u64 + }; + time_diff.partition_point(|t| *t < from_t) } else { 0 }; + let end_idx = if let Some(to) = to_time { - time_diff.partition_point(|t| *t <= to) + let to_t = if to < 0 { + time_diff + .last() + .map(|&last_time_diff| last_time_diff.saturating_sub(to.abs() as u64)) + .unwrap_or(0) + } else { + to as u64 + }; + time_diff.partition_point(|t| *t <= to_t) } else { time_diff.len() }; - (start_idx, end_idx) + + (start_idx, end_idx.max(start_idx)) } #[cfg(test)] @@ -553,6 +571,93 @@ mod tests { assert_eq!(compute_time_diff_index(&td, Some(25), Some(74)), (25, 75)); } + // ---- negative index tests ---- + + #[test] + fn test_index_negative_from() { + // time_diff: [0, 10, 20, 30, 40], last=40 + // from=-15 → 40-15=25 → first index where t >= 25 is index 3 (value 30) + assert_eq!( + compute_time_diff_index(&[0, 10, 20, 30, 40], Some(-15), None), + (3, 5) + ); + } + + #[test] + fn test_index_negative_to() { + // time_diff: [0, 10, 20, 30, 40], last=40 + // to=-15 → 40-15=25 → first index where t > 25 is index 3 (value 30), so end=3 + assert_eq!( + compute_time_diff_index(&[0, 10, 20, 30, 40], None, Some(-15)), + (0, 3) + ); + } + + #[test] + fn test_index_both_negative() { + // time_diff: [0, 10, 20, 30, 40], last=40 + // from=-30 → 40-30=10, to=-10 → 40-10=30 + // range [10, 30] → indices 1..4 + assert_eq!( + compute_time_diff_index(&[0, 10, 20, 30, 40], Some(-30), Some(-10)), + (1, 4) + ); + } + + #[test] + fn test_index_negative_from_positive_to() { + // time_diff: [0, 10, 20, 30, 40], last=40 + // from=-25 → 40-25=15, to=30 + // range [15, 30] → indices 2..4 + assert_eq!( + compute_time_diff_index(&[0, 10, 20, 30, 40], Some(-25), Some(30)), + (2, 4) + ); + } + + #[test] + fn test_index_negative_exceeds_range() { + // time_diff: [0, 10, 20], last=20 + // from=-100 → 20-100 saturates to 0 → full range from start + assert_eq!( + compute_time_diff_index(&[0, 10, 20], Some(-100), None), + (0, 3) + ); + } + + #[test] + fn test_index_negative_on_empty() { + assert_eq!(compute_time_diff_index(&[], Some(-5), Some(-1)), (0, 0)); + } + + #[test] + fn test_index_negative_second_by_second() { + // Typical: 100-second recording, want last 10 seconds + let td: Vec = (0..100).collect(); + // from=-10 → 99-10=89, to=-1 → 99-1=98 + // range [89, 98] → indices 89..99 + assert_eq!(compute_time_diff_index(&td, Some(-10), Some(-1)), (89, 99)); + } + + #[test] + fn test_index_inverted_range_returns_empty() { + // from=35 (positive), to=-30 → resolves to 40-30=10 + // Resolved range [35, 10] is inverted → should return empty (start, start) + assert_eq!( + compute_time_diff_index(&[0, 10, 20, 30, 40], Some(35), Some(-30)), + (4, 4) + ); + } + + #[test] + fn test_index_inverted_positive_range_returns_empty() { + // Pure positive inversion: from=30, to=10 + assert_eq!( + compute_time_diff_index(&[0, 10, 20, 30, 40], Some(30), Some(10)), + (3, 3) + ); + } + // ---- iterator tests ---- #[test] diff --git a/src/report.rs b/src/report.rs index f335241f..493202ec 100644 --- a/src/report.rs +++ b/src/report.rs @@ -42,9 +42,9 @@ struct RunsInfo { /// The map from run names to paths to the run data directory (where data are available for read and processing) run_dir_paths: HashMap, /// The specified start time of every run's time range - per_run_from_time: HashMap, + per_run_from_time: HashMap, /// The specified end time of every run's time range - per_run_to_time: HashMap, + per_run_to_time: HashMap, } impl RunsInfo { @@ -95,7 +95,7 @@ impl RunsInfo { fn process_per_run_time_range( &mut self, - run_time_ranges: &Vec<(String, Option, Option)>, + run_time_ranges: &Vec<(String, Option, Option)>, ) -> Result<()> { for (run_name, from_time, to_time) in run_time_ranges { // Empty run name means apply to all runs @@ -113,7 +113,8 @@ impl RunsInfo { }; if let (Some(from_time), Some(to_time)) = (from_time, to_time) { - if from_time > to_time { + // Quickly fail if the two bounds are of the same sign and FROM > TO + if (*from_time ^ *to_time) >= 0 && *from_time > *to_time { return Err(PDError::InvalidRunTimeRangeOption(format!( "The specified from_time {} is larger than to_time {} for run {}.", from_time, @@ -169,25 +170,27 @@ pub struct Report { #[clap(help_heading = "Basic Options", short, long, value_parser)] pub name: Option, - /// Time range to apply to a run in the report. All the time-series metrics, stats, and + /// The time range to apply to a run in the report. All the time-series metrics, stats, and /// analytical findings of the run will be limited to the specified time range. /// =================================== /// Format: RUN_NAME=FROM:TO or FROM:TO /// =================================== /// where FROM and TO are in seconds from the start of the run. If no run name is specified, - /// the time range is applied to all runs. Either bound can be omitted and it can be specified - /// for multiple runs. - /// Example: --time-range first_run=10:60 second_run=:30 + /// the time range is applied to all runs. Either bound can be omitted or negative, and it + /// can be specified for multiple runs. + /// Example: --time-range first_run=10:60 --time-range second_run=:30 /// --time-range 20:150 + /// --time-range -10:-5 #[clap( help_heading = "Basic Options", verbatim_doc_comment, long, value_parser = parse_time_range, value_name = "RUN=FROM:TO", - num_args = 1.. + allow_hyphen_values = true, + num_args = 1 )] - pub time_range: Vec<(String, Option, Option)>, + pub time_range: Vec<(String, Option, Option)>, } pub fn report(report: &Report, tmp_dir: &PathBuf) -> Result<()> { @@ -260,7 +263,7 @@ pub fn report(report: &Report, tmp_dir: &PathBuf) -> Result<()> { /// Used to parse the --time-range option, in the format of run_name=from_time:to_time, /// into a tuple (run_name, from_time, to_time) -fn parse_time_range(s: &str) -> Result<(String, Option, Option), String> { +fn parse_time_range(s: &str) -> Result<(String, Option, Option), String> { // If there's no '=', treat the whole string as FROM:TO (applies to all runs) let (run_name, range) = s.split_once('=').unwrap_or(("", s)); let (from_str, to_str) = range @@ -272,7 +275,7 @@ fn parse_time_range(s: &str) -> Result<(String, Option, Option), Strin } else { Some( from_str - .parse::() + .parse::() .map_err(|e| format!("invalid FROM value: {}", e))?, ) }; @@ -281,7 +284,7 @@ fn parse_time_range(s: &str) -> Result<(String, Option, Option), Strin } else { Some( to_str - .parse::() + .parse::() .map_err(|e| format!("invalid TO value: {}", e))?, ) }; diff --git a/tests/test_aperf.rs b/tests/test_aperf.rs index 10a846c9..67f7b2fd 100644 --- a/tests/test_aperf.rs +++ b/tests/test_aperf.rs @@ -546,14 +546,19 @@ fn test_duplicate_run_names() { } #[test] -fn test_report_with_time_range() { +fn test_report_with_time_ranges() { run_test(|work_dir, tmp_dir| { - let run_name = String::from("test_run_1"); - let run_path = get_test_data_path(format!("{}.tar.gz", run_name)); + let run_name_1 = String::from("test_run_1"); + let run_path_1 = get_test_data_path(format!("{run_name_1}.tar.gz")); + let run_name_2 = String::from("test_run_2"); + let run_path_2 = get_test_data_path(format!("{run_name_2}.tar.gz")); let report_name = String::from("time_range_report"); let rep = Report { - run: vec![run_path.into_os_string().into_string().unwrap()], + run: vec![ + run_path_1.into_os_string().into_string().unwrap(), + run_path_2.into_os_string().into_string().unwrap(), + ], name: Some( work_dir .join(&report_name) @@ -561,26 +566,14 @@ fn test_report_with_time_range() { .into_string() .unwrap(), ), - time_range: vec![(run_name.clone(), Some(5), Some(30))], + time_range: vec![ + (run_name_1.clone(), Some(5), Some(30)), + (run_name_2.clone(), Some(-8), Some(-2)), + ], }; assert!(report(&rep, &tmp_dir).is_ok()); - verify_report_structure(&work_dir, &report_name, vec![run_name]); - - // Verify the processed JS files exist and contain valid JSON - let js_dir = work_dir.join(&report_name).join("data").join("js"); - for entry in fs::read_dir(&js_dir).unwrap() { - let entry = entry.unwrap(); - let path = entry.path(); - if path.extension().map_or(false, |ext| ext == "js") { - let content = fs::read_to_string(&path).unwrap(); - assert!( - !content.is_empty(), - "JS file {:?} should not be empty", - path - ); - } - } + verify_report_structure(&work_dir, &report_name, vec![run_name_1, run_name_2]); clean_dir_and_archive(&work_dir, &report_name);