Add deterministic Callgrind benchmarking harness + periodic CI workflow for libgraphql-parser#104
Open
jeffmo wants to merge 6 commits into
Open
Add deterministic Callgrind benchmarking harness + periodic CI workflow for libgraphql-parser#104jeffmo wants to merge 6 commits into
jeffmo wants to merge 6 commits into
Conversation
The existing Criterion benchmarks measure wall-clock time, which is only meaningful on quiet dedicated hardware — they cannot run usefully on shared CI runners where scheduling noise, CPU frequency scaling, and noisy neighbors swamp real differences. This adds a second bench target (callgrind_benchmarks) built on iai-callgrind, which runs each benchmark on Valgrind's simulated CPU and records exact instruction counts plus simulated cache hits/misses (and a derived estimated- cycles metric). Repeated runs of the same binary produce bit-identical counts, making the suite suitable for CI regression tracking and for periodic cross-parser comparisons against graphql-parser and apollo-parser. Design decisions: - The suite mirrors the Criterion fixtures (synthetic small/medium/ large schemas, Star Wars, GitHub, Shopify Admin, plus executable documents) so results are comparable across both suites. Because instruction counts are deterministic, no separate "standalone" vs "compare" groups are needed — one measurement serves both purposes. - Fixture text is resolved in iai-callgrind `setup` functions, so file I/O and fixture generation are excluded from the measured region. - `--cache-sim=yes` is passed explicitly rather than relying on defaults so results stay comparable across iai-callgrind versions. - Valgrind only supports Linux, so the iai-callgrind dev-dependency is target-gated and the bench target compiles to an explanatory stub elsewhere (notably macOS on Apple Silicon, where the Criterion suite remains the local option). Verified end-to-end in a Linux container (valgrind 3.22): the full suite runs in ~7 minutes and repeated runs produce identical counts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018gi8GkWL2qnq3gw6cQP5FN
Companion to the callgrind_benchmarks bench target: verifies prerequisites (Linux, valgrind, a version-matched iai-callgrind- runner, the locally-fetched Shopify fixture) with actionable error messages, runs the suite with --save-summary=json, and formats the per-benchmark summary.json files into markdown comparison tables (instructions and estimated cycles, per fixture, across libgraphql- parser, lean mode, graphql-parser, and apollo-parser). The report is written to target/iai/REPORT.md so CI can publish it to the workflow summary page. A --format-only flag reformats an existing target/iai directory without re-running the benchmarks. Lean mode is excluded from best-value bolding since it does strictly less work than the other parsers' default configurations. Follows the structure and formatting conventions of the existing run-benchmarks.sh (Criterion) script. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018gi8GkWL2qnq3gw6cQP5FN
Adds a "Parser benchmarks" GitHub Actions workflow that runs the deterministic Callgrind suite on ubuntu-latest, on demand (workflow_dispatch) and on a monthly schedule. Because the suite measures simulated-CPU instruction counts rather than wall-clock time, results from shared GitHub runners are reproducible and comparable across runs. By default (and always for scheduled runs) the workflow first bumps graphql-parser and apollo-parser to their latest stable crates.io releases — resolved via the crates.io sparse index and pinned exactly in the transient CI checkout — so each report answers "how does libgraphql-parser compare against what users would install today?" rather than against the versions pinned in Cargo.lock. A dispatch input can disable the bump to benchmark against the pinned versions instead. If a new competitor release breaks the benchmark build, the run fails visibly, which is the desired signal that the comparison code needs updating. The markdown report is published to the workflow run's summary page and the raw iai-callgrind summary.json files are uploaded as a 90-day artifact for offline comparison between runs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018gi8GkWL2qnq3gw6cQP5FN
Extends the Performance section of the libgraphql-parser README with a subsection explaining what the Callgrind suite measures, why it exists alongside the wall-clock Criterion suite (deterministic results on noisy CI runners), how to run it locally, and how the periodic CI workflow compares against the latest crates.io releases of competing parsers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018gi8GkWL2qnq3gw6cQP5FN
Fixes findings from a code review of the Callgrind benchmark harness: - read_metric() now handles both summary.json metric shapes: `Left` (first-ever run) and `Both` (any run with previous data under target/iai, which iai-callgrind diffs against by default). Previously every re-run without clearing target/iai silently degraded the whole report to N/A. Verified by re-running the suite on top of existing results. - find_min_idx() no longer word-splits its arguments through echo/awk, which dropped empty fields and could bold the wrong parser's column whenever a metric was missing for a non-last parser. Rewritten as a pure-bash loop where empty values still occupy an index. - graphql-parser is now additionally benchmarked in its zero-copy `&str` mode. The owned-`String` mode (kept for comparability with the Criterion suite) forces a heap allocation for every identifier, which overstates graphql-parser's cost relative to the other parsers' zero-copy behavior. Reporting both modes sidesteps picking a single "fair" configuration. - The report header now carries a fidelity caveat explaining that the parsers produce different levels of output (lossless CST vs semantic-only AST) and which column pairs are the fairest single-axis comparisons. - latest_stable_crate_version() sorts sparse-index versions with sort --version-sort instead of assuming publish order equals semver order (a backport release would previously win). - Error/success glyphs now use the shared UNICODE_RED_X / UNICODE_GREEN_CHECK variables from scripts/_include.sh, and the competitor-bump script documents that graphql-parser is also a regular dependency of other workspace crates plus the regex- literal assumption on crate names. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018gi8GkWL2qnq3gw6cQP5FN
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Two fixes for CI findings on PR #104: - The cargo-build-test-clippy job failed on a collapsible_match lint in libgraphql-core-v1's SchemaBuilder (pre-existing code from main; CI runs a newer clippy than was used when it merged). Applied clippy's suggested fix: the empty-enum check moves from a nested `if` into a match-arm guard. Behavior is unchanged — non-empty enums fall through to the wildcard arm exactly as before — and the existing visit_enum_with_no_values_is_an_error test covers the path. - CodeQL flagged the new parser-benchmarks workflow for not limiting GITHUB_TOKEN permissions. Added an explicit least-privilege `permissions: contents: read` block; the job only reads the repo and uploads an artifact. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018gi8GkWL2qnq3gw6cQP5FN
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a second, fully deterministic benchmark suite for
libgraphql-parserthat runs under Valgrind's Callgrind (via iai-callgrind), plus a GitHub Actions workflow to run it periodically in CI against the latest crates.io releases of competing parsers.Callgrind executes each benchmark on a simulated CPU and counts exact instructions plus simulated cache hits/misses (deriving an "estimated cycles" metric). Repeated runs of the same binary produce bit-identical counts — verified locally — so results are meaningful even on GitHub's noisy shared runners, where wall-clock (Criterion) benchmarks are not. The Criterion suite remains the source of headline wall-clock numbers on quiet bare metal.
What's included
benches/callgrind_benchmarks.rs— mirrors the Criterion fixtures (synthetic small/medium/large schemas, Star Wars, GitHub, Shopify Admin, plus executable documents) across:libgraphql-parser(default + lean config)graphql-parser(owned-Stringmode, matching the Criterion suite, and zero-copy&strmode)apollo-parserFixture loading happens in iai-callgrind
setupfunctions, excluded from measurement. Linux-gated (Valgrind doesn't support macOS on Apple Silicon); compiles to an explanatory stub elsewhere.scripts/run-callgrind-benchmarks.sh— prerequisite checks with actionable errors, runs the suite (~7 min for all 43 benchmarks), and formats results into markdown comparison tables (written totarget/iai/REPORT.md), including a fidelity caveat distinguishing lossless-CST parsers from semantic-AST-only parsers..github/workflows/parser-benchmarks.yml—workflow_dispatch+ monthly cron. By default it first bumpsgraphql-parser/apollo-parserto their latest stable crates.io releases (resolved via the sparse index; transient to the CI checkout, never committed). The report publishes to the workflow run's summary page; rawsummary.jsondata uploads as a 90-day artifact.README documentation for the new suite.
Sample results (this container, valgrind 3.22, rustc 1.94.1)
Schema parsing, instructions (lower is better):
libgraphql-parsergraphql-parserapollo-parserReview
A
graphql-rust-reviewerpass was run over the diff; its findings (a jq path that silently degraded reports to N/A on local re-runs, a word-splitting bug in min-value bolding, and the owned-Stringfairness issue forgraphql-parser) are fixed in the final commit.Notes
main.graphql-parsercomparison to zero-copy mode too (left untouched here to preserve longitudinal comparability with the B1–B24 optimization history).🤖 Generated with Claude Code
https://claude.ai/code/session_018gi8GkWL2qnq3gw6cQP5FN
Generated by Claude Code