From 63ee3817a70fb854f3bb6d520755f32ed8ee8d43 Mon Sep 17 00:00:00 2001 From: Brian Picciano <933154+mediocregopher@users.noreply.github.com> Date: Mon, 20 Apr 2026 11:21:32 +0000 Subject: [PATCH 1/3] revert(engine): revert #23541 and #23578 This reverts commit 93b2201c7606ce5a9ee1c4fc898b22ad4e9c8f97 and commit cf29b3fffece1c20434873b91b69ba94ffb31952. Co-authored-by: Brian Picciano <933154+mediocregopher@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019daa9e-42a5-74da-a301-2dcd4474bf17 Co-authored-by: Amp --- .github/scripts/bench-reth-summary.py | 33 ++------------------ .github/scripts/bench-utils.js | 1 - crates/engine/primitives/src/message.rs | 8 +---- crates/engine/tree/src/tree/metrics.rs | 10 +++--- crates/engine/tree/src/tree/mod.rs | 14 ++------- crates/engine/tree/src/tree/tests.rs | 1 - crates/engine/util/src/reorg.rs | 7 ++--- crates/engine/util/src/skip_new_payload.rs | 4 +-- examples/bsc-p2p/src/block_import/service.rs | 2 +- 9 files changed, 16 insertions(+), 64 deletions(-) diff --git a/.github/scripts/bench-reth-summary.py b/.github/scripts/bench-reth-summary.py index 300006cad6c..2a66b84b95c 100755 --- a/.github/scripts/bench-reth-summary.py +++ b/.github/scripts/bench-reth-summary.py @@ -111,14 +111,6 @@ def compute_stats(combined: list[dict]) -> dict: wall_clock_s = sum(total_latencies_ms) / 1_000 mean_total_lat_ms = sum(total_latencies_ms) / n - # Persistence wait mean (for main table) - persist_values_ms = [] - for r in combined: - v = r.get("persistence_wait_us") - if v is not None: - persist_values_ms.append(v / 1_000) - mean_persist_ms = sum(persist_values_ms) / len(persist_values_ms) if persist_values_ms else 0.0 - return { "n": n, "mean_ms": mean_lat, @@ -129,7 +121,6 @@ def compute_stats(combined: list[dict]) -> dict: "mean_mgas_s": mean_mgas_s, "wall_clock_s": wall_clock_s, "mean_total_lat_ms": mean_total_lat_ms, - "mean_persist_ms": mean_persist_ms, } @@ -154,7 +145,7 @@ def compute_wait_stats(combined: list[dict], field: str) -> dict: def _paired_data( baseline: list[dict], feature: list[dict] -) -> tuple[list[tuple[float, float]], list[float], list[float], list[float], list[float]]: +) -> tuple[list[tuple[float, float]], list[float], list[float], list[float]]: """Match blocks and return paired latencies and per-block diffs. Returns: @@ -162,7 +153,6 @@ def _paired_data( lat_diffs_ms: list of feature − baseline latency diffs in ms mgas_diffs: list of feature − baseline Mgas/s diffs total_lat_diffs_ms: list of feature − baseline total latency diffs in ms - persist_diffs_ms: list of feature − baseline persistence wait diffs in ms """ baseline_by_block = {r["block_number"]: r for r in baseline} feature_by_block = {r["block_number"]: r for r in feature} @@ -172,7 +162,6 @@ def _paired_data( lat_diffs_ms = [] mgas_diffs = [] total_lat_diffs_ms = [] - persist_diffs_ms = [] for bn in common_blocks: b = baseline_by_block[bn] f = feature_by_block[bn] @@ -190,10 +179,7 @@ def _paired_data( total_lat_diffs_ms.append( f["total_latency_us"] / 1_000 - b["total_latency_us"] / 1_000 ) - b_persist = (b.get("persistence_wait_us") or 0) / 1_000 - f_persist = (f.get("persistence_wait_us") or 0) / 1_000 - persist_diffs_ms.append(f_persist - b_persist) - return pairs, lat_diffs_ms, mgas_diffs, total_lat_diffs_ms, persist_diffs_ms + return pairs, lat_diffs_ms, mgas_diffs, total_lat_diffs_ms def compute_paired_stats( @@ -209,15 +195,13 @@ def compute_paired_stats( all_lat_diffs = [] all_mgas_diffs = [] all_total_lat_diffs = [] - all_persist_diffs = [] blocks_per_pair = [] for baseline, feature in zip(baseline_runs, feature_runs): - pairs, lat_diffs, mgas_diffs, total_lat_diffs, persist_diffs = _paired_data(baseline, feature) + pairs, lat_diffs, mgas_diffs, total_lat_diffs = _paired_data(baseline, feature) all_pairs.extend(pairs) all_lat_diffs.extend(lat_diffs) all_mgas_diffs.extend(mgas_diffs) all_total_lat_diffs.extend(total_lat_diffs) - all_persist_diffs.extend(persist_diffs) blocks_per_pair.append(len(pairs)) if not all_lat_diffs: @@ -261,11 +245,6 @@ def compute_paired_stats( total_se = std_total_diff / math.sqrt(len(all_total_lat_diffs)) if all_total_lat_diffs else 0.0 wall_clock_ci_ms = T_CRITICAL * total_se - mean_persist_diff = sum(all_persist_diffs) / len(all_persist_diffs) if all_persist_diffs else 0.0 - std_persist_diff = stddev(all_persist_diffs, mean_persist_diff) if len(all_persist_diffs) > 1 else 0.0 - persist_se = std_persist_diff / math.sqrt(len(all_persist_diffs)) if all_persist_diffs else 0.0 - persist_ci_ms = T_CRITICAL * persist_se - return { "n": n, "mean_diff_ms": mean_diff, @@ -279,7 +258,6 @@ def compute_paired_stats( "mean_mgas_diff": mean_mgas_diff, "mgas_ci": mgas_ci, "wall_clock_ci_ms": wall_clock_ci_ms, - "persist_ci_ms": persist_ci_ms, "blocks": max(blocks_per_pair), } @@ -358,7 +336,6 @@ def ci_pct(ci_ms: float, base_ms: float) -> float: ("p99", "p99_ms", "p99_ci_ms", "p99_ms", True), ("mgas_s", "mean_mgas_s", "mgas_ci", "mean_mgas_s", False), ("wall_clock", "wall_clock_s", "wall_clock_ci_ms", "mean_total_lat_ms", True), - ("persist_wait", "mean_persist_ms", "persist_ci_ms", "mean_persist_ms", True), ] changes = {} for name, stat_key, ci_key, base_key, lower_is_better in metrics: @@ -400,8 +377,6 @@ def pct(base: float, feat: float) -> float: p90_pct = pct(run1["p90_ms"], run2["p90_ms"]) p99_pct = pct(run1["p99_ms"], run2["p99_ms"]) - persist_pct = pct(run1["mean_persist_ms"], run2["mean_persist_ms"]) - # Bootstrap CIs as % of baseline percentile p50_ci_pct = paired["p50_ci_ms"] / run1["p50_ms"] * 100.0 if run1["p50_ms"] > 0 else 0.0 p90_ci_pct = paired["p90_ci_ms"] / run1["p90_ms"] * 100.0 if run1["p90_ms"] > 0 else 0.0 @@ -411,7 +386,6 @@ def pct(base: float, feat: float) -> float: lat_ci_pct = paired["ci_ms"] / run1["mean_ms"] * 100.0 if run1["mean_ms"] > 0 else 0.0 mgas_ci_pct = paired["mgas_ci"] / run1["mean_mgas_s"] * 100.0 if run1["mean_mgas_s"] > 0 else 0.0 wall_ci_pct = paired["wall_clock_ci_ms"] / run1["mean_total_lat_ms"] * 100.0 if run1["mean_total_lat_ms"] > 0 else 0.0 - persist_ci_pct = paired["persist_ci_ms"] / run1["mean_persist_ms"] * 100.0 if run1["mean_persist_ms"] > 0 else 0.0 base_url = f"https://github.com/{repo}/commit" baseline_label = f"[`{baseline_name}`]({base_url}/{baseline_ref})" @@ -427,7 +401,6 @@ def pct(base: float, feat: float) -> float: f"| P99 | {fmt_ms(run1['p99_ms'])} | {fmt_ms(run2['p99_ms'])} | {change_str(p99_pct, p99_ci_pct, lower_is_better=True)} |", f"| Mgas/s | {fmt_mgas(run1['mean_mgas_s'])} | {fmt_mgas(run2['mean_mgas_s'])} | {change_str(gas_pct, mgas_ci_pct, lower_is_better=False)} |", f"| Wall Clock | {fmt_s(run1['wall_clock_s'])} | {fmt_s(run2['wall_clock_s'])} | {change_str(wall_pct, wall_ci_pct, lower_is_better=True)} |", - f"| Persist Wait | {fmt_ms(run1['mean_persist_ms'])} | {fmt_ms(run2['mean_persist_ms'])} | {change_str(persist_pct, persist_ci_pct, lower_is_better=True)} |", "", ] meta_parts = [f"{n} {'big blocks' if big_blocks else 'blocks'}"] diff --git a/.github/scripts/bench-utils.js b/.github/scripts/bench-utils.js index 393c402fb25..92d4a286a14 100644 --- a/.github/scripts/bench-utils.js +++ b/.github/scripts/bench-utils.js @@ -83,7 +83,6 @@ function metricRows(summary) { { label: 'P99', baseline: fmtMs(b.p99_ms), feature: fmtMs(f.p99_ms), change: fmtChange(c.p99) }, { label: 'Mgas/s', baseline: fmtMgas(b.mean_mgas_s), feature: fmtMgas(f.mean_mgas_s), change: fmtChange(c.mgas_s) }, { label: 'Wall Clock', baseline: fmtS(b.wall_clock_s), feature: fmtS(f.wall_clock_s), change: fmtChange(c.wall_clock) }, - { label: 'Persist Wait', baseline: fmtMs(b.mean_persist_ms || 0), feature: fmtMs(f.mean_persist_ms || 0), change: fmtChange(c.persist_wait) }, ]; } diff --git a/crates/engine/primitives/src/message.rs b/crates/engine/primitives/src/message.rs index fca83328777..430beee3f1b 100644 --- a/crates/engine/primitives/src/message.rs +++ b/crates/engine/primitives/src/message.rs @@ -196,8 +196,6 @@ pub enum BeaconEngineMessage { payload: Payload::ExecutionData, /// The sender for returning payload status result. tx: oneshot::Sender>, - /// When this message was enqueued, used to measure backpressure wait time. - enqueued_at: Instant, }, /// Message with new payload used by `reth_newPayload` endpoint. /// @@ -290,11 +288,7 @@ where payload: Payload::ExecutionData, ) -> Result { let (tx, rx) = oneshot::channel(); - let _ = self.to_engine.send(BeaconEngineMessage::NewPayload { - payload, - tx, - enqueued_at: Instant::now(), - }); + let _ = self.to_engine.send(BeaconEngineMessage::NewPayload { payload, tx }); rx.await.map_err(|_| BeaconOnNewPayloadError::EngineUnavailable)? } diff --git a/crates/engine/tree/src/tree/metrics.rs b/crates/engine/tree/src/tree/metrics.rs index e9e0f593287..f8a96e3c6bb 100644 --- a/crates/engine/tree/src/tree/metrics.rs +++ b/crates/engine/tree/src/tree/metrics.rs @@ -436,9 +436,9 @@ impl NewPayloadStatusMetrics { latest_forkchoice_updated_at: &mut Option, result: &Result, InsertBlockFatalError>, gas_used: u64, - latency: Duration, ) { let finish = Instant::now(); + let elapsed = finish - start; if let Some(prev_finish) = self.latest_finish_at { self.time_between_new_payloads.record(start - prev_finish); @@ -455,13 +455,13 @@ impl NewPayloadStatusMetrics { if !outcome.already_seen { self.new_payload_total_gas.record(gas_used as f64); self.new_payload_total_gas_last.set(gas_used as f64); - let gas_per_second = gas_used as f64 / latency.as_secs_f64(); + let gas_per_second = gas_used as f64 / elapsed.as_secs_f64(); self.new_payload_gas_per_second.record(gas_per_second); self.new_payload_gas_per_second_last.set(gas_per_second); - self.new_payload_latency.record(latency); - self.new_payload_last.set(latency); - self.gas_bucket.record(gas_used, latency); + self.new_payload_latency.record(elapsed); + self.new_payload_last.set(elapsed); + self.gas_bucket.record(gas_used, elapsed); } } PayloadStatusEnum::Syncing => self.new_payload_syncing.increment(1), diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs index e7188f75016..e5230db98b6 100644 --- a/crates/engine/tree/src/tree/mod.rs +++ b/crates/engine/tree/src/tree/mod.rs @@ -1608,18 +1608,16 @@ where warn!(target: "engine::tree", ?state, elapsed=?start.elapsed(), "Failed to deliver forkchoiceUpdated response, receiver dropped (request cancelled): {err:?}"); } } - BeaconEngineMessage::NewPayload { payload, tx, enqueued_at } => { + BeaconEngineMessage::NewPayload { payload, tx } => { let start = Instant::now(); let gas_used = payload.gas_used(); let num_hash = payload.num_hash(); let mut output = self.on_new_payload(payload); - let latency = enqueued_at.elapsed(); self.metrics.engine.new_payload.update_response_metrics( start, &mut self.metrics.engine.forkchoice_updated.latest_finish_at, &output, gas_used, - latency, ); let maybe_event = @@ -1695,20 +1693,12 @@ where let gas_used = payload.gas_used(); let num_hash = payload.num_hash(); let mut output = self.on_new_payload(payload); - - // Latency measures time from enqueue to completion, excluding - // only the explicit persistence wait. This means backpressure - // (time spent queued due to the engine being busy) is included, - // reflecting real-world engine responsiveness. - let latency = - enqueued_at.elapsed().saturating_sub(explicit_persistence_wait); - + let latency = start.elapsed(); self.metrics.engine.new_payload.update_response_metrics( start, &mut self.metrics.engine.forkchoice_updated.latest_finish_at, &output, gas_used, - latency, ); let maybe_event = diff --git a/crates/engine/tree/src/tree/tests.rs b/crates/engine/tree/src/tree/tests.rs index 23dcdd217cc..bbb6c4c59b1 100644 --- a/crates/engine/tree/src/tree/tests.rs +++ b/crates/engine/tree/src/tree/tests.rs @@ -682,7 +682,6 @@ async fn test_holesky_payload() { sidecar: ExecutionPayloadSidecar::none(), }, tx, - enqueued_at: std::time::Instant::now(), } .into(), )) diff --git a/crates/engine/util/src/reorg.rs b/crates/engine/util/src/reorg.rs index 8d33559a5a0..adfec89ba0e 100644 --- a/crates/engine/util/src/reorg.rs +++ b/crates/engine/util/src/reorg.rs @@ -25,7 +25,6 @@ use std::{ future::Future, pin::Pin, task::{ready, Context, Poll}, - time::Instant, }; use tokio::sync::oneshot; use tracing::*; @@ -144,7 +143,7 @@ where let next = ready!(this.stream.poll_next_unpin(cx)); let item = match (next, &this.last_forkchoice_state) { ( - Some(BeaconEngineMessage::NewPayload { payload, tx, enqueued_at }), + Some(BeaconEngineMessage::NewPayload { payload, tx }), Some(last_forkchoice_state), ) if this.forkchoice_states_forwarded > this.frequency && // Only enter reorg state if new payload attaches to current head. @@ -174,7 +173,6 @@ where return Poll::Ready(Some(BeaconEngineMessage::NewPayload { payload, tx, - enqueued_at, })) } }; @@ -193,12 +191,11 @@ where let queue = VecDeque::from([ // Current payload - BeaconEngineMessage::NewPayload { payload, tx, enqueued_at }, + BeaconEngineMessage::NewPayload { payload, tx }, // Reorg payload BeaconEngineMessage::NewPayload { payload: T::block_to_payload(reorg_block), tx: reorg_payload_tx, - enqueued_at: Instant::now(), }, // Reorg forkchoice state BeaconEngineMessage::ForkchoiceUpdated { diff --git a/crates/engine/util/src/skip_new_payload.rs b/crates/engine/util/src/skip_new_payload.rs index 22c637d5d58..fa818f54538 100644 --- a/crates/engine/util/src/skip_new_payload.rs +++ b/crates/engine/util/src/skip_new_payload.rs @@ -41,7 +41,7 @@ where loop { let next = ready!(this.stream.poll_next_unpin(cx)); let item = match next { - Some(BeaconEngineMessage::NewPayload { payload, tx, enqueued_at }) => { + Some(BeaconEngineMessage::NewPayload { payload, tx }) => { if this.skipped < this.threshold { *this.skipped += 1; tracing::warn!( @@ -56,7 +56,7 @@ where continue } *this.skipped = 0; - Some(BeaconEngineMessage::NewPayload { payload, tx, enqueued_at }) + Some(BeaconEngineMessage::NewPayload { payload, tx }) } next => next, }; diff --git a/examples/bsc-p2p/src/block_import/service.rs b/examples/bsc-p2p/src/block_import/service.rs index 63d368e979c..4e4029d7e95 100644 --- a/examples/bsc-p2p/src/block_import/service.rs +++ b/examples/bsc-p2p/src/block_import/service.rs @@ -413,7 +413,7 @@ mod tests { tokio::spawn(async move { while let Some(message) = from_engine.recv().await { match message { - BeaconEngineMessage::NewPayload { payload: _, tx, .. } => { + BeaconEngineMessage::NewPayload { payload: _, tx } => { tx.send(Ok(PayloadStatus::new(responses.new_payload.clone(), None))) .unwrap(); } From 21bd00641458944b49bce140f3e884a2d8e197cd Mon Sep 17 00:00:00 2001 From: Brian Picciano <933154+mediocregopher@users.noreply.github.com> Date: Mon, 20 Apr 2026 11:28:35 +0000 Subject: [PATCH 2/3] fix(bench): keep persist wait in summary table Keep the persist wait row and paired significance data in the benchmark summaries while leaving the reverted latency timing behavior unchanged. Co-authored-by: Brian Picciano <933154+mediocregopher@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019daa9e-42a5-74da-a301-2dcd4474bf17 Co-authored-by: Amp --- .github/scripts/bench-reth-summary.py | 31 ++++++++++++++++++++++++--- .github/scripts/bench-utils.js | 3 ++- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/.github/scripts/bench-reth-summary.py b/.github/scripts/bench-reth-summary.py index 2a66b84b95c..7de85c24231 100755 --- a/.github/scripts/bench-reth-summary.py +++ b/.github/scripts/bench-reth-summary.py @@ -111,6 +111,13 @@ def compute_stats(combined: list[dict]) -> dict: wall_clock_s = sum(total_latencies_ms) / 1_000 mean_total_lat_ms = sum(total_latencies_ms) / n + persist_values_ms = [] + for r in combined: + v = r.get("persistence_wait_us") + if v is not None: + persist_values_ms.append(v / 1_000) + mean_persist_ms = sum(persist_values_ms) / len(persist_values_ms) if persist_values_ms else 0.0 + return { "n": n, "mean_ms": mean_lat, @@ -121,6 +128,7 @@ def compute_stats(combined: list[dict]) -> dict: "mean_mgas_s": mean_mgas_s, "wall_clock_s": wall_clock_s, "mean_total_lat_ms": mean_total_lat_ms, + "mean_persist_ms": mean_persist_ms, } @@ -145,7 +153,7 @@ def compute_wait_stats(combined: list[dict], field: str) -> dict: def _paired_data( baseline: list[dict], feature: list[dict] -) -> tuple[list[tuple[float, float]], list[float], list[float], list[float]]: +) -> tuple[list[tuple[float, float]], list[float], list[float], list[float], list[float]]: """Match blocks and return paired latencies and per-block diffs. Returns: @@ -153,6 +161,7 @@ def _paired_data( lat_diffs_ms: list of feature − baseline latency diffs in ms mgas_diffs: list of feature − baseline Mgas/s diffs total_lat_diffs_ms: list of feature − baseline total latency diffs in ms + persist_diffs_ms: list of feature − baseline persistence wait diffs in ms """ baseline_by_block = {r["block_number"]: r for r in baseline} feature_by_block = {r["block_number"]: r for r in feature} @@ -162,6 +171,7 @@ def _paired_data( lat_diffs_ms = [] mgas_diffs = [] total_lat_diffs_ms = [] + persist_diffs_ms = [] for bn in common_blocks: b = baseline_by_block[bn] f = feature_by_block[bn] @@ -179,7 +189,10 @@ def _paired_data( total_lat_diffs_ms.append( f["total_latency_us"] / 1_000 - b["total_latency_us"] / 1_000 ) - return pairs, lat_diffs_ms, mgas_diffs, total_lat_diffs_ms + b_persist = (b.get("persistence_wait_us") or 0) / 1_000 + f_persist = (f.get("persistence_wait_us") or 0) / 1_000 + persist_diffs_ms.append(f_persist - b_persist) + return pairs, lat_diffs_ms, mgas_diffs, total_lat_diffs_ms, persist_diffs_ms def compute_paired_stats( @@ -195,13 +208,15 @@ def compute_paired_stats( all_lat_diffs = [] all_mgas_diffs = [] all_total_lat_diffs = [] + all_persist_diffs = [] blocks_per_pair = [] for baseline, feature in zip(baseline_runs, feature_runs): - pairs, lat_diffs, mgas_diffs, total_lat_diffs = _paired_data(baseline, feature) + pairs, lat_diffs, mgas_diffs, total_lat_diffs, persist_diffs = _paired_data(baseline, feature) all_pairs.extend(pairs) all_lat_diffs.extend(lat_diffs) all_mgas_diffs.extend(mgas_diffs) all_total_lat_diffs.extend(total_lat_diffs) + all_persist_diffs.extend(persist_diffs) blocks_per_pair.append(len(pairs)) if not all_lat_diffs: @@ -245,6 +260,11 @@ def compute_paired_stats( total_se = std_total_diff / math.sqrt(len(all_total_lat_diffs)) if all_total_lat_diffs else 0.0 wall_clock_ci_ms = T_CRITICAL * total_se + mean_persist_diff = sum(all_persist_diffs) / len(all_persist_diffs) if all_persist_diffs else 0.0 + std_persist_diff = stddev(all_persist_diffs, mean_persist_diff) if len(all_persist_diffs) > 1 else 0.0 + persist_se = std_persist_diff / math.sqrt(len(all_persist_diffs)) if all_persist_diffs else 0.0 + persist_ci_ms = T_CRITICAL * persist_se + return { "n": n, "mean_diff_ms": mean_diff, @@ -258,6 +278,7 @@ def compute_paired_stats( "mean_mgas_diff": mean_mgas_diff, "mgas_ci": mgas_ci, "wall_clock_ci_ms": wall_clock_ci_ms, + "persist_ci_ms": persist_ci_ms, "blocks": max(blocks_per_pair), } @@ -336,6 +357,7 @@ def ci_pct(ci_ms: float, base_ms: float) -> float: ("p99", "p99_ms", "p99_ci_ms", "p99_ms", True), ("mgas_s", "mean_mgas_s", "mgas_ci", "mean_mgas_s", False), ("wall_clock", "wall_clock_s", "wall_clock_ci_ms", "mean_total_lat_ms", True), + ("persist_wait", "mean_persist_ms", "persist_ci_ms", "mean_persist_ms", True), ] changes = {} for name, stat_key, ci_key, base_key, lower_is_better in metrics: @@ -376,6 +398,7 @@ def pct(base: float, feat: float) -> float: p50_pct = pct(run1["p50_ms"], run2["p50_ms"]) p90_pct = pct(run1["p90_ms"], run2["p90_ms"]) p99_pct = pct(run1["p99_ms"], run2["p99_ms"]) + persist_pct = pct(run1["mean_persist_ms"], run2["mean_persist_ms"]) # Bootstrap CIs as % of baseline percentile p50_ci_pct = paired["p50_ci_ms"] / run1["p50_ms"] * 100.0 if run1["p50_ms"] > 0 else 0.0 @@ -386,6 +409,7 @@ def pct(base: float, feat: float) -> float: lat_ci_pct = paired["ci_ms"] / run1["mean_ms"] * 100.0 if run1["mean_ms"] > 0 else 0.0 mgas_ci_pct = paired["mgas_ci"] / run1["mean_mgas_s"] * 100.0 if run1["mean_mgas_s"] > 0 else 0.0 wall_ci_pct = paired["wall_clock_ci_ms"] / run1["mean_total_lat_ms"] * 100.0 if run1["mean_total_lat_ms"] > 0 else 0.0 + persist_ci_pct = paired["persist_ci_ms"] / run1["mean_persist_ms"] * 100.0 if run1["mean_persist_ms"] > 0 else 0.0 base_url = f"https://github.com/{repo}/commit" baseline_label = f"[`{baseline_name}`]({base_url}/{baseline_ref})" @@ -401,6 +425,7 @@ def pct(base: float, feat: float) -> float: f"| P99 | {fmt_ms(run1['p99_ms'])} | {fmt_ms(run2['p99_ms'])} | {change_str(p99_pct, p99_ci_pct, lower_is_better=True)} |", f"| Mgas/s | {fmt_mgas(run1['mean_mgas_s'])} | {fmt_mgas(run2['mean_mgas_s'])} | {change_str(gas_pct, mgas_ci_pct, lower_is_better=False)} |", f"| Wall Clock | {fmt_s(run1['wall_clock_s'])} | {fmt_s(run2['wall_clock_s'])} | {change_str(wall_pct, wall_ci_pct, lower_is_better=True)} |", + f"| Persist Wait | {fmt_ms(run1['mean_persist_ms'])} | {fmt_ms(run2['mean_persist_ms'])} | {change_str(persist_pct, persist_ci_pct, lower_is_better=True)} |", "", ] meta_parts = [f"{n} {'big blocks' if big_blocks else 'blocks'}"] diff --git a/.github/scripts/bench-utils.js b/.github/scripts/bench-utils.js index 92d4a286a14..52a27b3c5ed 100644 --- a/.github/scripts/bench-utils.js +++ b/.github/scripts/bench-utils.js @@ -69,7 +69,7 @@ function blocksLabel(summary) { return parts; } -// The 7 metric rows shared by all renderers. +// The metric rows shared by all renderers. // Returns an array of { label, baseline, feature, change } objects. function metricRows(summary) { const b = summary.baseline.stats; @@ -83,6 +83,7 @@ function metricRows(summary) { { label: 'P99', baseline: fmtMs(b.p99_ms), feature: fmtMs(f.p99_ms), change: fmtChange(c.p99) }, { label: 'Mgas/s', baseline: fmtMgas(b.mean_mgas_s), feature: fmtMgas(f.mean_mgas_s), change: fmtChange(c.mgas_s) }, { label: 'Wall Clock', baseline: fmtS(b.wall_clock_s), feature: fmtS(f.wall_clock_s), change: fmtChange(c.wall_clock) }, + { label: 'Persist Wait', baseline: fmtMs(b.mean_persist_ms || 0), feature: fmtMs(f.mean_persist_ms || 0), change: fmtChange(c.persist_wait) }, ]; } From e19202452d456c8ff8081e4041d84fccdb506104 Mon Sep 17 00:00:00 2001 From: Brian Picciano <933154+mediocregopher@users.noreply.github.com> Date: Mon, 20 Apr 2026 11:32:08 +0000 Subject: [PATCH 3/3] chore(bench): trim cosmetic script diffs Restore the original comments and spacing so the persist wait summary change stays minimal. Co-authored-by: Brian Picciano <933154+mediocregopher@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019daa9e-42a5-74da-a301-2dcd4474bf17 Co-authored-by: Amp --- .github/scripts/bench-reth-summary.py | 2 ++ .github/scripts/bench-utils.js | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/scripts/bench-reth-summary.py b/.github/scripts/bench-reth-summary.py index 7de85c24231..300006cad6c 100755 --- a/.github/scripts/bench-reth-summary.py +++ b/.github/scripts/bench-reth-summary.py @@ -111,6 +111,7 @@ def compute_stats(combined: list[dict]) -> dict: wall_clock_s = sum(total_latencies_ms) / 1_000 mean_total_lat_ms = sum(total_latencies_ms) / n + # Persistence wait mean (for main table) persist_values_ms = [] for r in combined: v = r.get("persistence_wait_us") @@ -398,6 +399,7 @@ def pct(base: float, feat: float) -> float: p50_pct = pct(run1["p50_ms"], run2["p50_ms"]) p90_pct = pct(run1["p90_ms"], run2["p90_ms"]) p99_pct = pct(run1["p99_ms"], run2["p99_ms"]) + persist_pct = pct(run1["mean_persist_ms"], run2["mean_persist_ms"]) # Bootstrap CIs as % of baseline percentile diff --git a/.github/scripts/bench-utils.js b/.github/scripts/bench-utils.js index 52a27b3c5ed..393c402fb25 100644 --- a/.github/scripts/bench-utils.js +++ b/.github/scripts/bench-utils.js @@ -69,7 +69,7 @@ function blocksLabel(summary) { return parts; } -// The metric rows shared by all renderers. +// The 7 metric rows shared by all renderers. // Returns an array of { label, baseline, feature, change } objects. function metricRows(summary) { const b = summary.baseline.stats;