From 0e92e23de6cd677e1da858ed7913761d2f0e8e91 Mon Sep 17 00:00:00 2001 From: jedarden Date: Mon, 6 Jul 2026 11:22:18 -0400 Subject: [PATCH] feat(bf-54zad): implement benchmark timing and throughput measurement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add files_per_second field to BenchmarkResult struct - Implement calculate_files_per_second() method - Implement save_to_file() method for JSON baseline recording - Update run_benchmark() to calculate files_per_second metric - Update main() to display and save files_per_second Acceptance criteria: ✅ Timing code measures wall-clock time accurately (Instant::now) ✅ Throughput (MB/s) calculated correctly (bytes_total / duration) ✅ Files per second calculated correctly (files_total / duration_sec) ✅ Commit hash extraction works (git rev-parse HEAD) ✅ Timestamp produces ISO 8601 (chrono::Utc::now().to_rfc3339) ✅ Ready to output metrics to JSON (save_to_file method) See notes/bf-54zad.md for verification details. --- .needle-predispatch-sha | 2 +- crates/pdftract-cli/benches/grep_1000.rs | 71 ++++++++++++++++- notes/bf-54zad.md | 99 ++++++++++++++++++++++++ 3 files changed, 167 insertions(+), 5 deletions(-) create mode 100644 notes/bf-54zad.md diff --git a/.needle-predispatch-sha b/.needle-predispatch-sha index f2d5448..87a4b71 100644 --- a/.needle-predispatch-sha +++ b/.needle-predispatch-sha @@ -1 +1 @@ -e65ff59581844e03b71f5af54e22028dcd7e6f7c +2e07f2568cc1478d86f345b01cd3f2bbe394960c diff --git a/crates/pdftract-cli/benches/grep_1000.rs b/crates/pdftract-cli/benches/grep_1000.rs index 894b38a..075ebae 100644 --- a/crates/pdftract-cli/benches/grep_1000.rs +++ b/crates/pdftract-cli/benches/grep_1000.rs @@ -99,6 +99,8 @@ struct BenchmarkResult { matches_total: usize, /// Throughput in MB/s throughput_mb_s: f64, + /// Files processed per second + files_per_second: f64, /// Peak RSS in MB peak_rss_mb: Option, } @@ -113,6 +115,15 @@ impl BenchmarkResult { bytes_per_sec / (1024.0 * 1024.0) } + /// Calculate files per second + fn calculate_files_per_second(&self) -> f64 { + if self.duration_ms == 0 { + return 0.0; + } + let duration_sec = self.duration_ms as f64 / 1000.0; + self.files_total as f64 / duration_sec + } + /// Validate against CI gates fn validate(&self) -> Result<(), String> { // During development (files_total == 0), skip validation @@ -132,6 +143,38 @@ impl BenchmarkResult { Ok(()) } + + /// Save benchmark result to JSON file + /// + /// Creates a file named `.json` in the benches/results directory. + fn save_to_file(&self) -> Result<(), std::io::Error> { + use std::fs; + use std::io::Write; + + // Create results directory if it doesn't exist + let results_dir = PathBuf::from("benches/results"); + fs::create_dir_all(&results_dir)?; + + // Create filename from commit SHA (short form, 8 chars) + let commit_short = if self.commit.len() > 8 { + &self.commit[..8] + } else { + &self.commit + }; + let filename = format!("{}.json", commit_short); + let filepath = results_dir.join(&filename); + + // Serialize to JSON + let json_str = serde_json::to_string_pretty(self)?; + + // Write to file + let mut file = fs::File::create(&filepath)?; + file.write_all(json_str.as_bytes())?; + file.flush()?; + + eprintln!("Benchmark result saved to: {}", filepath.display()); + Ok(()) + } } /// Get current git commit SHA @@ -214,7 +257,7 @@ fn run_benchmark() -> Result { eprintln!("Run tests/fixtures/grep-corpus/regenerate.sh to populate the corpus."); // Return a placeholder result that won't fail CI gates - return Ok(BenchmarkResult { + let placeholder = BenchmarkResult { commit: get_commit_sha(), started_at: chrono::Utc::now().to_rfc3339(), files_total: 0, @@ -222,8 +265,10 @@ fn run_benchmark() -> Result { duration_ms: 0, matches_total: 0, throughput_mb_s: 0.0, + files_per_second: 0.0, peak_rss_mb: None, - }); + }; + return Ok(placeholder); } eprintln!( @@ -251,8 +296,19 @@ fn run_benchmark() -> Result { bytes_total, duration_ms, matches_total, - throughput_mb_s: 0.0, // Calculated below - peak_rss_mb: None, // TODO: measure via /usr/bin/time -v or rusage + throughput_mb_s: 0.0, // Will be calculated below + files_per_second: 0.0, // Will be calculated below + peak_rss_mb: None, // TODO: measure via /usr/bin/time -v or rusage + }; + + // Calculate derived metrics + let throughput = result.calculate_throughput(); + let files_per_sec = result.calculate_files_per_second(); + + let result = BenchmarkResult { + throughput_mb_s: throughput, + files_per_second: files_per_sec, + ..result }; // Validate against gates @@ -300,6 +356,13 @@ fn main() { Ok(result) => { println!("{:#?}", result); println!("\nThroughput: {:.2} MB/s", result.calculate_throughput()); + println!("Files/sec: {:.2}", result.calculate_files_per_second()); + + // Save to JSON file for baseline recording + if let Err(e) = result.save_to_file() { + eprintln!("Warning: Failed to save results to file: {}", e); + } + println!("All CI gates passed!"); } Err(e) => { diff --git a/notes/bf-54zad.md b/notes/bf-54zad.md new file mode 100644 index 0000000..ef95854 --- /dev/null +++ b/notes/bf-54zad.md @@ -0,0 +1,99 @@ +# bf-54zad: Benchmark Timing and Throughput Measurement + +## Summary + +Implemented timing instrumentation and throughput measurement logic for the grep benchmark. + +## Changes Made + +### File: `crates/pdftract-cli/benches/grep_1000.rs` + +1. **Added `files_per_second` field to `BenchmarkResult` struct** (line 103) + - Stores the calculated files per second metric + +2. **Implemented `calculate_files_per_second()` method** (lines 118-125) + - Calculates files/second: `files_total / duration_sec` + - Handles division by zero (returns 0.0 if duration_ms == 0) + - Formula: `files_total / (duration_ms / 1000.0)` + +3. **Implemented `save_to_file()` method** (lines 147-180) + - Creates `benches/results/` directory if it doesn't exist + - Saves benchmark result as `.json` (8-char short SHA) + - Uses `serde_json::to_string_pretty()` for readable JSON output + - Provides user feedback on file save location + +4. **Updated `run_benchmark()` function** (lines 260-280) + - Calculates `files_per_second` metric after benchmark execution + - Creates updated `BenchmarkResult` with calculated metrics + +5. **Updated `main()` function** (lines 354-374) + - Displays `files_per_second` metric alongside throughput + - Calls `save_to_file()` to persist results to JSON + - Provides warning if file save fails (non-fatal) + +## Acceptance Criteria Status + +| Criterion | Status | Notes | +|-----------|--------|-------| +| Timing code measures wall-clock time accurately | ✅ PASS | Uses `Instant::now()` and `.elapsed().as_millis()` | +| Throughput (MB/s) calculated correctly | ✅ PASS | `calculate_throughput()` method: `(bytes * 1000) / duration_ms / (1024*1024)` | +| Files per second calculated correctly | ✅ PASS | `calculate_files_per_second()` method: `files / (duration_ms / 1000)` | +| Commit hash extraction works | ✅ PASS | `get_commit_sha()` uses `git rev-parse HEAD` | +| Timestamp generation produces ISO 8601 | ✅ PASS | Uses `chrono::Utc::now().to_rfc3339()` | +| Ready to output metrics to JSON | ✅ PASS | `save_to_file()` method outputs to `benches/results/.json` | + +## Technical Details + +### Wall-clock timing +- Uses `std::time::Instant::now()` for start time +- Measures elapsed milliseconds with `.elapsed().as_millis()` +- High-resolution monotonic clock suitable for benchmarking + +### Throughput calculation (MB/s) +```rust +let bytes_per_sec = (bytes_total as f64 * 1000.0) / duration_ms as f64; +bytes_per_sec / (1024.0 * 1024.0) +``` +- Converts duration from ms to seconds +- Calculates bytes per second +- Divides by 1024² for binary MB (MiB/s) + +### Files per second calculation +```rust +let duration_sec = duration_ms as f64 / 1000.0; +files_total as f64 / duration_sec +``` +- Simple linear scaling from files per duration_ms to files per second + +### JSON output format +```json +{ + "commit": "abc12345...", + "started_at": "2026-07-06T10:30:45+00:00", + "files_total": 1000, + "bytes_total": 104857600, + "duration_ms": 2000, + "matches_total": 5000, + "throughput_mb_s": 50.0, + "files_per_second": 500.0, + "peak_rss_mb": null +} +``` + +## Testing + +### Compile check +```bash +cargo check --bench grep_1000 +``` +✅ No compilation errors + +### Manual test (corpus not yet populated) +The benchmark handles empty corpus gracefully - returns placeholder result and skips CI gate validation during development. + +## Next Steps + +The benchmark infrastructure is now ready. Once the grep subcommand is complete (7.8.x beads): +1. Populate `tests/fixtures/grep-corpus/` with 1000 PDFs +2. Wire up actual grep execution +3. Run full benchmark and validate 50 MB/s CI gate