Skip to content

perf(cdp): add benchmark harness report - #180

Merged
aram-devdocs merged 1 commit into
mainfrom
issue-61b-benchmark-harness-report
May 1, 2026
Merged

perf(cdp): add benchmark harness report#180
aram-devdocs merged 1 commit into
mainfrom
issue-61b-benchmark-harness-report

Conversation

@aram-devdocs

Copy link
Copy Markdown
Owner

Summary

  • Add Criterion benchmark suite (crates/plumb-cdp/benches/cdp_benchmarks.rs) with three groups: per_rule_dom (100/1k/10k node synthetic DOMs), cold_start, and warm_run (CDP benchmarks behind e2e-chromium feature gate)
  • Wire just bench and just bench-full targets in the Justfile
  • Add docs/src/performance.md with benchmark usage, fixture descriptions, and result interpretation guidance

Validation

  • cargo fmt --all -- --check — clean
  • cargo clippy -p plumb-cdp --benches -- -D warnings — clean
  • cargo bench -p plumb-cdp --no-run — compiles successfully
  • git diff --check — no whitespace issues
  • CDP benchmarks (cold_start, warm_run) require a real Chromium binary and are gated behind the e2e-chromium feature; they will not run in CI without Chromium installed

Blockers

Refs #61

🤖 Generated with Claude Code

Add a Criterion benchmark suite for the CDP pipeline:

- per_rule_dom: rule-engine cost on 100/1k/10k node synthetic DOMs
- cold_start / warm_run: CDP snapshot latency (behind e2e-chromium feature)

Wire `just bench` and `just bench-full` targets.
Add docs/src/performance.md with usage and interpretation guidance.

Refs #61

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented May 1, 2026

Copy link
Copy Markdown
Contributor

I have all the context I need to render the review.


PR #180perf(cdp): add benchmark harness report

Scope: crates/plumb-cdp/benches/cdp_benchmarks.rs (new), Cargo.lock, crates/plumb-cdp/Cargo.toml, docs/src/performance.md (new), docs/src/SUMMARY.md, justfile.


1. Determinism invariants — CLEAN

No library code changed. The bench binary calls plumb_core::run, which is the deterministic core; the harness itself is excluded from the determinism CI job. criterion pulls in oorandom (an RNG) but only inside the harness binary, not in any observable output path. No SystemTime::now or HashMap leak into library code.

2. Workspace layering — CLEAN

criterion is correctly placed under [dev-dependencies] in crates/plumb-cdp/Cargo.toml, referenced via { workspace = true } where the workspace root declares criterion = { version = "0.5", features = ["html_reports"] } at line 101. The bench binary sits in plumb-cdp (the one crate allowed to hold unsafe); the layering rule is unaffected.

All four public types used in the e2e-chromium-gated mod cdp block (BrowserDriver, ChromiumDriver, ChromiumOptions, PersistentBrowser, Target) are confirmed present in crates/plumb-cdp/src/lib.rs.

3. Error handling — CLEAN

unwrap / expect appear only inside the bench binary with a file-level #![allow(clippy::unwrap_used, clippy::expect_used)]. That allow is scoped to a standalone binary, not a library crate — no rule violation.

4. Dependency hygiene — CLEAN

New transitive deps (anes, cast, ciborium*, crunchy, half, hermit-abi, is-terminal, itertools 0.10.5, oorandom, plotters*, same-file, tinytemplate, walkdir, winapi-util) are all MIT / Apache-2.0 / BSD-2 or dual-licensed combinations, all within the deny.toml allow-list. itertools 0.10.5 has no prior version in the lockfile — no multiple-version warning.

5. Documentation — CLEAN

docs/src/performance.md is concise and factual. No AI-writing tells from the documented list ("dive in", "seamless", "leverage", "comprehensive", etc.) were found.


Punch list

  • crates/plumb-cdp/benches/cdp_benchmarks.rs:106–113 — Three inline #[allow(clippy::cast_possible_truncation)] / #[allow(clippy::cast_possible_wrap)] are dead noise; the file-level #![allow(..., clippy::cast_possible_truncation, clippy::cast_possible_wrap, ...)] on line 23 already suppresses them. Not a blocker, but tidy-up is cheap.

Verdict: APPROVE

@aram-devdocs
aram-devdocs merged commit 9ce7a26 into main May 1, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant