From 3176fd90ac5c3154088977b1c34ef0c1db327b20 Mon Sep 17 00:00:00 2001 From: Matt Currier Date: Fri, 17 Apr 2026 11:09:40 -0400 Subject: [PATCH 1/2] fix: Rename throughput fields from mbps to mb_s for correctness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The throughput fields were named average_throughput_mbps and peak_throughput_mbps, which conventionally means "megabits per second." However, the calculation divides bytes_per_second by 1,000,000, producing megabytes per second (MB/s). This rename corrects the field names to match the actual unit. Changes: - Rename average_throughput_mbps → average_throughput_mb_s - Rename peak_throughput_mbps → peak_throughput_mb_s - Update doc comments from "megabits" to "megabytes per second (MB/s)" - Remove workaround comments that acknowledged the mismatch - Update JSON example in README.md - All tests passing, clippy clean Fixes: #112 BREAKING CHANGE: Serialized JSON field names changed from *_mbps to *_mb_s. Downstream consumers must update accordingly. AI-assisted-by: Claude Opus 4 (Cursor agent) Made-with: Cursor --- README.md | 2 +- src/results.rs | 36 +++++++++++++++++------------------- src/results_blocking.rs | 16 +++++++--------- 3 files changed, 25 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index 827c6dd7..495bf4d0 100644 --- a/README.md +++ b/README.md @@ -658,7 +658,7 @@ The benchmark generates comprehensive JSON output with the following structure: "summary": { "total_messages_sent": 10000, "total_bytes_transferred": 10240000, - "average_throughput_mbps": 305.17, + "average_throughput_mb_s": 305.17, "p95_latency_ns": 5200, "p99_latency_ns": 8500 } diff --git a/src/results.rs b/src/results.rs index 064b554b..e448420a 100644 --- a/src/results.rs +++ b/src/results.rs @@ -368,11 +368,11 @@ pub struct BenchmarkSummary { /// Total number of bytes transferred during all tests pub total_bytes_transferred: usize, - /// Average throughput across all tests in megabits per second - pub average_throughput_mbps: f64, + /// Average throughput across all tests in megabytes per second (MB/s) + pub average_throughput_mb_s: f64, - /// Peak throughput observed in any single test in megabits per second - pub peak_throughput_mbps: f64, + /// Peak throughput observed in any single test in megabytes per second (MB/s) + pub peak_throughput_mb_s: f64, /// Average latency across all latency measurements (if any) pub average_latency_ns: Option, @@ -1133,7 +1133,7 @@ impl ResultsManager { result.mechanism.to_string(), MechanismSummary { mechanism: result.mechanism, - average_throughput_mbps: result.summary.average_throughput_mbps, + average_throughput_mb_s: result.summary.average_throughput_mb_s, p95_latency_ns: result.summary.p95_latency_ns, p99_latency_ns: result.summary.p99_latency_ns, total_messages: result.summary.total_messages_sent, @@ -1163,7 +1163,7 @@ impl ResultsManager { /// /// ## Comparison Method /// - /// Comparison is based on average throughput in megabits per second, + /// Comparison is based on average throughput in megabytes per second, /// which provides a consistent measure across different message sizes /// and test configurations. fn find_fastest_mechanism(&self) -> Option { @@ -1171,8 +1171,8 @@ impl ResultsManager { .iter() .max_by(|a, b| { a.summary - .average_throughput_mbps - .partial_cmp(&b.summary.average_throughput_mbps) + .average_throughput_mb_s + .partial_cmp(&b.summary.average_throughput_mb_s) .unwrap() }) .map(|result| result.mechanism.to_string()) @@ -1380,11 +1380,9 @@ impl ResultsManager { let summary = &result.summary; println!("{}Throughput:", indent); - // Note: The summary field is named _mbps, but the calculation in update_summary - // produces MB/s (Megabytes per second). The label reflects the calculation. println!( "{}{:<8} Average: {:.2} MB/s, Peak: {:.2} MB/s", - indent, " ", summary.average_throughput_mbps, summary.peak_throughput_mbps + indent, " ", summary.average_throughput_mb_s, summary.peak_throughput_mb_s ); println!("{}Totals:", indent); @@ -1525,8 +1523,8 @@ pub struct MechanismSummary { /// The IPC mechanism this summary describes pub mechanism: IpcMechanism, - /// Average throughput performance in megabits per second - pub average_throughput_mbps: f64, + /// Average throughput performance in megabytes per second (MB/s) + pub average_throughput_mb_s: f64, /// 95th percentile latency (if latency was measured) pub p95_latency_ns: Option, @@ -1691,9 +1689,9 @@ impl BenchmarkResults { } // Calculate summary metrics - let average_throughput_mbps = + let average_throughput_mb_s = throughput_values.iter().sum::() / throughput_values.len() as f64 / 1_000_000.0; - let peak_throughput_mbps = + let peak_throughput_mb_s = throughput_values.iter().cloned().fold(0.0, f64::max) / 1_000_000.0; // Calculate properly weighted average latency across all test types @@ -1705,8 +1703,8 @@ impl BenchmarkResults { self.summary = BenchmarkSummary { total_messages_sent: total_messages, total_bytes_transferred: total_bytes, - average_throughput_mbps, - peak_throughput_mbps, + average_throughput_mb_s, + peak_throughput_mb_s, average_latency_ns, min_latency_ns, max_latency_ns, @@ -1851,8 +1849,8 @@ impl Default for BenchmarkSummary { Self { total_messages_sent: 0, total_bytes_transferred: 0, - average_throughput_mbps: 0.0, - peak_throughput_mbps: 0.0, + average_throughput_mb_s: 0.0, + peak_throughput_mb_s: 0.0, average_latency_ns: None, min_latency_ns: None, max_latency_ns: None, diff --git a/src/results_blocking.rs b/src/results_blocking.rs index 20edc8d2..fa65408c 100644 --- a/src/results_blocking.rs +++ b/src/results_blocking.rs @@ -1005,7 +1005,7 @@ impl BlockingResultsManager { result.mechanism.to_string(), MechanismSummary { mechanism: result.mechanism, - average_throughput_mbps: result.summary.average_throughput_mbps, + average_throughput_mb_s: result.summary.average_throughput_mb_s, p95_latency_ns: result.summary.p95_latency_ns, p99_latency_ns: result.summary.p99_latency_ns, total_messages: result.summary.total_messages_sent, @@ -1035,7 +1035,7 @@ impl BlockingResultsManager { /// /// ## Comparison Method /// - /// Comparison is based on average throughput in megabits per second, + /// Comparison is based on average throughput in megabytes per second, /// which provides a consistent measure across different message sizes /// and test configurations. fn find_fastest_mechanism(&self) -> Option { @@ -1043,8 +1043,8 @@ impl BlockingResultsManager { .iter() .max_by(|a, b| { a.summary - .average_throughput_mbps - .partial_cmp(&b.summary.average_throughput_mbps) + .average_throughput_mb_s + .partial_cmp(&b.summary.average_throughput_mb_s) .unwrap() }) .map(|result| result.mechanism.to_string()) @@ -1261,11 +1261,9 @@ impl BlockingResultsManager { let summary = &result.summary; println!("{}Throughput:", indent); - // Note: The summary field is named _mbps, but the calculation in update_summary - // produces MB/s (Megabytes per second). The label reflects the calculation. println!( "{}{:<8} Average: {:.2} MB/s, Peak: {:.2} MB/s", - indent, " ", summary.average_throughput_mbps, summary.peak_throughput_mbps + indent, " ", summary.average_throughput_mb_s, summary.peak_throughput_mb_s ); println!("{}Totals:", indent); @@ -2191,7 +2189,7 @@ mod tests { // Add results with different throughputs let mut results1 = create_test_results(); - results1.summary.average_throughput_mbps = 100.0; + results1.summary.average_throughput_mb_s = 100.0; manager.add_results(results1).unwrap(); // Create a second result with higher throughput @@ -2206,7 +2204,7 @@ mod tests { true, true, ); - results2.summary.average_throughput_mbps = 200.0; // Higher + results2.summary.average_throughput_mb_s = 200.0; // Higher manager.add_results(results2).unwrap(); // The fastest should be SharedMemory From d19121365962ab5ecd6955f90704891e76d2e5d6 Mon Sep 17 00:00:00 2001 From: Matt Currier Date: Fri, 17 Apr 2026 16:51:52 -0400 Subject: [PATCH 2/2] fix: Use fully verbose throughput field names per review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename average_throughput_mb_s → average_throughput_megabytes_per_sec and peak_throughput_mb_s → peak_throughput_megabytes_per_sec for unambiguous field naming in serialized output. AI-assisted-by: Claude Opus 4 (Cursor agent) Made-with: Cursor --- README.md | 2 +- src/results.rs | 31 ++++++++++++++++++------------- src/results_blocking.rs | 17 +++++++++++------ 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 495bf4d0..8795ff48 100644 --- a/README.md +++ b/README.md @@ -658,7 +658,7 @@ The benchmark generates comprehensive JSON output with the following structure: "summary": { "total_messages_sent": 10000, "total_bytes_transferred": 10240000, - "average_throughput_mb_s": 305.17, + "average_throughput_megabytes_per_sec": 305.17, "p95_latency_ns": 5200, "p99_latency_ns": 8500 } diff --git a/src/results.rs b/src/results.rs index e448420a..edf0096e 100644 --- a/src/results.rs +++ b/src/results.rs @@ -369,10 +369,10 @@ pub struct BenchmarkSummary { pub total_bytes_transferred: usize, /// Average throughput across all tests in megabytes per second (MB/s) - pub average_throughput_mb_s: f64, + pub average_throughput_megabytes_per_sec: f64, /// Peak throughput observed in any single test in megabytes per second (MB/s) - pub peak_throughput_mb_s: f64, + pub peak_throughput_megabytes_per_sec: f64, /// Average latency across all latency measurements (if any) pub average_latency_ns: Option, @@ -1133,7 +1133,9 @@ impl ResultsManager { result.mechanism.to_string(), MechanismSummary { mechanism: result.mechanism, - average_throughput_mb_s: result.summary.average_throughput_mb_s, + average_throughput_megabytes_per_sec: result + .summary + .average_throughput_megabytes_per_sec, p95_latency_ns: result.summary.p95_latency_ns, p99_latency_ns: result.summary.p99_latency_ns, total_messages: result.summary.total_messages_sent, @@ -1171,8 +1173,8 @@ impl ResultsManager { .iter() .max_by(|a, b| { a.summary - .average_throughput_mb_s - .partial_cmp(&b.summary.average_throughput_mb_s) + .average_throughput_megabytes_per_sec + .partial_cmp(&b.summary.average_throughput_megabytes_per_sec) .unwrap() }) .map(|result| result.mechanism.to_string()) @@ -1382,7 +1384,10 @@ impl ResultsManager { println!("{}Throughput:", indent); println!( "{}{:<8} Average: {:.2} MB/s, Peak: {:.2} MB/s", - indent, " ", summary.average_throughput_mb_s, summary.peak_throughput_mb_s + indent, + " ", + summary.average_throughput_megabytes_per_sec, + summary.peak_throughput_megabytes_per_sec ); println!("{}Totals:", indent); @@ -1524,7 +1529,7 @@ pub struct MechanismSummary { pub mechanism: IpcMechanism, /// Average throughput performance in megabytes per second (MB/s) - pub average_throughput_mb_s: f64, + pub average_throughput_megabytes_per_sec: f64, /// 95th percentile latency (if latency was measured) pub p95_latency_ns: Option, @@ -1689,9 +1694,9 @@ impl BenchmarkResults { } // Calculate summary metrics - let average_throughput_mb_s = + let average_throughput_megabytes_per_sec = throughput_values.iter().sum::() / throughput_values.len() as f64 / 1_000_000.0; - let peak_throughput_mb_s = + let peak_throughput_megabytes_per_sec = throughput_values.iter().cloned().fold(0.0, f64::max) / 1_000_000.0; // Calculate properly weighted average latency across all test types @@ -1703,8 +1708,8 @@ impl BenchmarkResults { self.summary = BenchmarkSummary { total_messages_sent: total_messages, total_bytes_transferred: total_bytes, - average_throughput_mb_s, - peak_throughput_mb_s, + average_throughput_megabytes_per_sec, + peak_throughput_megabytes_per_sec, average_latency_ns, min_latency_ns, max_latency_ns, @@ -1849,8 +1854,8 @@ impl Default for BenchmarkSummary { Self { total_messages_sent: 0, total_bytes_transferred: 0, - average_throughput_mb_s: 0.0, - peak_throughput_mb_s: 0.0, + average_throughput_megabytes_per_sec: 0.0, + peak_throughput_megabytes_per_sec: 0.0, average_latency_ns: None, min_latency_ns: None, max_latency_ns: None, diff --git a/src/results_blocking.rs b/src/results_blocking.rs index fa65408c..1aaa2990 100644 --- a/src/results_blocking.rs +++ b/src/results_blocking.rs @@ -1005,7 +1005,9 @@ impl BlockingResultsManager { result.mechanism.to_string(), MechanismSummary { mechanism: result.mechanism, - average_throughput_mb_s: result.summary.average_throughput_mb_s, + average_throughput_megabytes_per_sec: result + .summary + .average_throughput_megabytes_per_sec, p95_latency_ns: result.summary.p95_latency_ns, p99_latency_ns: result.summary.p99_latency_ns, total_messages: result.summary.total_messages_sent, @@ -1043,8 +1045,8 @@ impl BlockingResultsManager { .iter() .max_by(|a, b| { a.summary - .average_throughput_mb_s - .partial_cmp(&b.summary.average_throughput_mb_s) + .average_throughput_megabytes_per_sec + .partial_cmp(&b.summary.average_throughput_megabytes_per_sec) .unwrap() }) .map(|result| result.mechanism.to_string()) @@ -1263,7 +1265,10 @@ impl BlockingResultsManager { println!("{}Throughput:", indent); println!( "{}{:<8} Average: {:.2} MB/s, Peak: {:.2} MB/s", - indent, " ", summary.average_throughput_mb_s, summary.peak_throughput_mb_s + indent, + " ", + summary.average_throughput_megabytes_per_sec, + summary.peak_throughput_megabytes_per_sec ); println!("{}Totals:", indent); @@ -2189,7 +2194,7 @@ mod tests { // Add results with different throughputs let mut results1 = create_test_results(); - results1.summary.average_throughput_mb_s = 100.0; + results1.summary.average_throughput_megabytes_per_sec = 100.0; manager.add_results(results1).unwrap(); // Create a second result with higher throughput @@ -2204,7 +2209,7 @@ mod tests { true, true, ); - results2.summary.average_throughput_mb_s = 200.0; // Higher + results2.summary.average_throughput_megabytes_per_sec = 200.0; // Higher manager.add_results(results2).unwrap(); // The fastest should be SharedMemory