Skip to content

Commit 50f21ca

Browse files
committed
refactor(bench): parse N times, remove detailed flag, always collect real timings
Major refactoring to fix legacy issues and improve accuracy: 1. Parse time now measured N times (like formatting): - Parse template `iterations` times and average the result - Previously: parsed once, reused same value for all sizes - Now: accurate parse timing with same stability as format timing 2. Removed legacy "detailed" flag entirely: - Was broken: only worked with iterations=1 - Created fake/uniform timing data when disabled - Latency statistics with dummy data were meaningless 3. Always collect real per-path timings: - Removed conditional timing collection (detailed && iterations==1) - Removed fake data generation (vec![avg_per_path; size]) - Now collects actual timings across all iterations: (size × iterations) samples - Provides accurate latency statistics with real variance 4. Always show latency statistics: - Removed "if detailed" check around statistics display - Users always see min/p50/p95/p99/max/stddev - Statistics now reflect real data, not uniform averages 5. Cleaned up code: - Removed unused_assignments warning (total_duration) - Updated pattern description: "Parse and format N paths with M iterations" - Simplified function signatures (no detailed parameter) - Removed detailed CLI flag and all related code Benefits: - More accurate parse time measurements - Real latency variance visible in all runs - Simpler code (23 lines added, 40 deleted) - No more misleading fake statistics - Consistent measurement approach for parse and format
1 parent a9b871f commit 50f21ca

File tree

1 file changed

+23
-40
lines changed

1 file changed

+23
-40
lines changed

src/bin/bench_throughput.rs

Lines changed: 23 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -300,21 +300,27 @@ impl TemplateSet {
300300
}
301301
}
302302

303-
/// Runs a benchmark for a single template with varying input sizes and detailed profiling
303+
/// Runs a benchmark for a single template with varying input sizes
304304
fn benchmark_template(
305305
_template_name: &str,
306306
template_str: &str,
307307
sizes: &[usize],
308308
iterations: usize,
309-
detailed: bool,
310309
) -> Result<Vec<BenchmarkResult>, Box<dyn std::error::Error>> {
311310
let generator = PathGenerator::new();
312311
let mut results = Vec::new();
313312

314-
// Parse template once
315-
let parse_start = Instant::now();
313+
// Parse template N times and average
314+
let mut total_parse_time = Duration::ZERO;
315+
for _ in 0..iterations {
316+
let parse_start = Instant::now();
317+
let _ = Template::parse(template_str)?;
318+
total_parse_time += parse_start.elapsed();
319+
}
320+
let avg_parse_time = total_parse_time / iterations as u32;
321+
322+
// Parse once for actual use
316323
let template = Template::parse(template_str)?;
317-
let parse_time = parse_start.elapsed();
318324

319325
for &size in sizes {
320326
// Generate N paths for this size
@@ -325,33 +331,22 @@ fn benchmark_template(
325331
let _ = template.format(path)?;
326332
}
327333

328-
// Measure: format all paths multiple times for stable measurements
329-
let mut total_duration = Duration::ZERO;
330-
let mut individual_times = Vec::new();
334+
// Measure: format all paths multiple times, collecting individual timings
335+
let mut all_individual_times = Vec::new();
331336

332337
for _ in 0..iterations {
333-
let start = Instant::now();
334338
for path in &paths {
335339
let format_start = Instant::now();
336340
let _ = template.format(path)?;
337-
if detailed && iterations == 1 {
338-
// Only collect individual times on single iteration runs
339-
individual_times.push(format_start.elapsed());
340-
}
341+
all_individual_times.push(format_start.elapsed());
341342
}
342-
total_duration += start.elapsed();
343343
}
344344

345-
// Average across iterations
345+
// Calculate total from all iterations
346+
let total_duration: Duration = all_individual_times.iter().sum();
346347
let avg_format_time = total_duration / iterations as u32;
347348

348-
// If not detailed mode, create dummy individual times for stats
349-
if !detailed || iterations > 1 {
350-
let avg_per_path = avg_format_time / size as u32;
351-
individual_times = vec![avg_per_path; size];
352-
}
353-
354-
let result = BenchmarkResult::new(size, parse_time, avg_format_time, individual_times);
349+
let result = BenchmarkResult::new(size, avg_parse_time, avg_format_time, all_individual_times);
355350

356351
results.push(result);
357352
}
@@ -477,7 +472,7 @@ fn print_progress_bar(current: usize, total: usize, template_name: &str) {
477472
stdout.flush().ok();
478473
}
479474

480-
fn print_template_results(template_name: &str, results: &[BenchmarkResult], detailed: bool) {
475+
fn print_template_results(template_name: &str, results: &[BenchmarkResult]) {
481476
print_section_header(&format!("Template: {}", template_name));
482477

483478
// Create results table with comfy-table
@@ -581,7 +576,7 @@ fn print_template_results(template_name: &str, results: &[BenchmarkResult], deta
581576
}
582577

583578
// Latency statistics for largest size
584-
if detailed && !results.is_empty() {
579+
if !results.is_empty() {
585580
let largest_result = results.last().unwrap();
586581

587582
// Latency statistics
@@ -723,7 +718,7 @@ fn output_json(
723718
fn main() {
724719
let matches = Command::new("String Pipeline Throughput Benchmark")
725720
.version(env!("CARGO_PKG_VERSION"))
726-
.about("Benchmarks batch processing throughput with varying input sizes and detailed profiling")
721+
.about("Benchmarks batch processing throughput with varying input sizes")
727722
.arg(
728723
Arg::new("sizes")
729724
.short('s')
@@ -740,13 +735,6 @@ fn main() {
740735
.help("Number of measurement iterations per size for stability")
741736
.default_value("50"),
742737
)
743-
.arg(
744-
Arg::new("detailed")
745-
.short('d')
746-
.long("detailed")
747-
.action(clap::ArgAction::SetTrue)
748-
.help("Enable detailed per-operation profiling and statistics"),
749-
)
750738
.arg(
751739
Arg::new("format")
752740
.short('f')
@@ -788,7 +776,6 @@ fn main() {
788776
.parse()
789777
.expect("Invalid iteration count");
790778

791-
let detailed = matches.get_flag("detailed");
792779
let format = matches.get_one::<String>("format").unwrap();
793780
let output_path = matches.get_one::<String>("output");
794781
let quiet = matches.get_flag("quiet");
@@ -804,7 +791,7 @@ fn main() {
804791
let _ = execute!(
805792
stdout,
806793
Print("Measuring batch processing performance with varying input sizes\n"),
807-
Print("Pattern: Parse once, format N paths individually\n\n"),
794+
Print("Pattern: Parse and format N paths with M iterations for stability\n\n"),
808795
SetForegroundColor(Color::Cyan),
809796
Print("Input sizes: "),
810797
ResetColor,
@@ -817,10 +804,6 @@ fn main() {
817804
ResetColor,
818805
Print(format!("{}\n", iterations)),
819806
SetForegroundColor(Color::Cyan),
820-
Print("Detailed profiling: "),
821-
ResetColor,
822-
Print(if detailed { "enabled\n" } else { "disabled\n" }),
823-
SetForegroundColor(Color::Cyan),
824807
Print("Output format: "),
825808
ResetColor,
826809
Print(format!("{}\n", format))
@@ -836,7 +819,7 @@ fn main() {
836819
print_progress_bar(idx + 1, total_templates, template_name);
837820
}
838821

839-
match benchmark_template(template_name, template_str, &sizes, iterations, detailed) {
822+
match benchmark_template(template_name, template_str, &sizes, iterations) {
840823
Ok(results) => {
841824
if !quiet {
842825
let mut stdout = io::stdout();
@@ -849,7 +832,7 @@ fn main() {
849832
ResetColor,
850833
Print(format!("Completed: {}\n", template_name))
851834
);
852-
print_template_results(template_name, &results, detailed);
835+
print_template_results(template_name, &results);
853836
} else {
854837
print_success(&format!("Benchmarking '{}'", template_name));
855838
}

0 commit comments

Comments
 (0)