parser-performance-regression

name	parser-performance-regression
description	Analyze rdbinsight parser benchmark regressions and performance-sensitive PRs. Use when parser benchmark results, CI comments, PR reviews, or code changes show possible slowdown; when a regression exceeds 5%; when a benchmark gate fails; or when deciding whether profiling, optimization, or ROI evidence is required.

Parser Performance Regression

Use this skill to make parser performance work evidence-driven. Treat benchmark regressions as product behavior, not as incidental CI noise.

Policy

• A parser benchmark slowdown above 30% is a hard failure. Do not approve or merge until it is fixed or the benchmark/gate is proven invalid.
• A slowdown above 5% requires investigation. Either improve it, or leave concrete evidence that further optimization has low ROI.
• A slowdown below 5% can be accepted when it is within noise or tied to a clearly justified correctness/maintainability change, but still mention any notable pattern.
• Do not rely on one local run when making a performance claim. Prefer CI artifacts for PR decisions, and use local benchmarks/profiling to explain or improve the result.
• If a new benchmark case has current data but no base data, fix the benchmark harness/reporting before judging the change.
• If a gate failure prevents summary/comment output, fix the feedback loop so the failed run still publishes benchmark artifacts, aggregate summary, and PR comment data.

Initial Triage

1. Read the PR benchmark comment and CI summary.
2. Confirm the comment links to the exact base commit, head commit, CI run, and comment workflow run.
3. Confirm the result table is complete:
   • base row count equals current row count
   • expected benchmark cases are present
   • no new case is missing a base median
4. Sort slowdowns descending and classify:
   • >30%: blocker
   • >5%: investigate
   • 0..5%: note if patterned or surprising
   • <0%: improvement
5. Compare related cases before optimizing. For example, if hash-metadata regresses but hash improves, focus on metadata-specific parsing rather than shared dispatch.

GitHub/CI Evidence

Use gh to inspect runs and comments. Prefer machine-readable output.

gh pr view <PR> --repo DCjanus/rdbinsight --json headRefOid,baseRefOid,statusCheckRollup
gh run view <CI_RUN_ID> --repo DCjanus/rdbinsight --log > /tmp/rdbinsight-ci.log
rg -n 'Unable to complete|parser benchmark slowdown|ERROR|error:' /tmp/rdbinsight-ci.log

Download parser benchmark artifacts when you need exact Criterion data:

rm -rf /tmp/rdbinsight-bench && mkdir -p /tmp/rdbinsight-bench
gh run download <CI_RUN_ID> --repo DCjanus/rdbinsight --pattern 'parser-benchmark-*' --dir /tmp/rdbinsight-bench
find /tmp/rdbinsight-bench -path '*/target/criterion-base/parser/parse_rdb/*/base/estimates.json' | wc -l
find /tmp/rdbinsight-bench -path '*/target/criterion-current/parser/parse_rdb/*/new/estimates.json' | wc -l

Verify the PR benchmark comment after workflow changes:

gh api repos/DCjanus/rdbinsight/issues/comments/<COMMENT_ID> \
  --jq '{updated_at, rows: (.body | split("\n") | map(select(startswith("| `parser/parse_rdb/"))) | length)}'

Local Benchmarking

Parser benchmarks live in benches/parser.rs.

Important environment variables:

• RDBINSIGHT_BENCH_PROFILES: comma-separated cases, such as hash-metadata,list-quicklist2,string-int
• RDBINSIGHT_BENCH_GENERATED_BYTES: synthetic RDB size, CI currently uses 16777216
• RDBINSIGHT_BENCH_SAMPLE_SIZE: CI uses 10
• RDBINSIGHT_BENCH_WARM_UP_SECS: CI uses 1
• RDBINSIGHT_BENCH_MEASUREMENT_SECS: CI uses 3
• RDBINSIGHT_BENCH_SAMPLING_MODE: CI uses flat
• RDBINSIGHT_BENCH_MAX_SLOWDOWN_PERCENT: CI uses 30

Targeted local run:

RDBINSIGHT_BENCH_PROFILES=hash-metadata \
RDBINSIGHT_BENCH_GENERATED_BYTES=16777216 \
RDBINSIGHT_BENCH_SAMPLE_SIZE=10 \
RDBINSIGHT_BENCH_WARM_UP_SECS=1 \
RDBINSIGHT_BENCH_MEASUREMENT_SECS=3 \
RDBINSIGHT_BENCH_SAMPLING_MODE=flat \
cargo +nightly bench --bench parser -- --noplot

For quick iteration, reduce generated bytes, but do not use reduced-size numbers as final PR evidence:

RDBINSIGHT_BENCH_PROFILES=hash-metadata \
RDBINSIGHT_BENCH_GENERATED_BYTES=4194304 \
cargo +nightly bench --bench parser -- --noplot

Profiling Workflow

Use profiling when a slowdown is above 5% and the hot path is not obvious from code review.

Recommended options:

• macOS: sample, Instruments, or cargo instruments if available.
• Linux: perf, cargo flamegraph, samply, or hyperfine plus profiler output.
• Criterion artifacts: target/criterion-current and target/criterion-base estimates for medians and throughput.

Keep profiling focused:

1. Run one or a small set of regressed profiles.
2. Compare against a related non-regressed profile.
3. Look for repeated parser initialization, allocation, decompression, copying, string conversion, or generic combinator overhead inside per-entry loops.
4. Prefer optimizations that remove work from inner loops while preserving chunk-safe NeedMoreData behavior.

Optimization Patterns

Prefer narrow parser hot-path fixes:

• Replace generic ReduceParser<Seq*<...>> chains with specialized state machines when a benchmark repeatedly parses small fixed fields.
• Use direct read_rdb_len, read_rdb_str, or StringEncodingParser::init_from_input when the data is already in the current buffer and rollback semantics are not needed beyond the current state boundary.
• Avoid allocation and string conversion in comparison/counting paths.
• Preserve chunk safety: if a parser may need more data after partial progress, keep enough state to resume without rereading consumed bytes.
• Keep correctness tests when changing parser behavior, especially integer encodings, EOF/trailing bytes, and malformed input.

Do not optimize by weakening validation, skipping bytes without validating required structure, or changing benchmark data to hide a regression.

ROI Evidence

If a >5% slowdown remains, document why further work is low ROI. Good evidence includes:

• profiler output showing time dominated by required work or external library behavior
• multiple CI/local runs showing high noise and no stable regression
• a targeted optimization attempt with benchmark before/after showing negligible gain
• a correctness or API improvement whose cost is small, bounded, and accepted

Weak evidence is not enough:

• “below 30%”
• “probably noise”
• “CI passed”
• one local run without artifact or profiler support

Feedback Loop

After any benchmark or workflow change:

1. Run local checks relevant to the edit, such as:
   • cargo +nightly fmt --check
   • cargo +nightly test -q --lib
   • cargo +nightly test -q --test parser_test
   • actionlint .github/workflows/ci.yml .github/workflows/parser-benchmark-comment.yml
   • node --check .github/actions/parser-benchmark/index.js
2. Push and let CI produce benchmark artifacts.
3. Verify:
   • all parser benchmark shard jobs ran
   • aggregate summary ran even when a benchmark gate fails
   • benchmark comment updated automatically
   • comment row count matches artifact row count
   • no Criterion “Unable to complete 10 samples” warning remains unless explicitly accepted
4. Update PR title/body only from final net diff. Mention performance gate, benchmark coverage, and any remaining >5% slowdown rationale.