Skip to content

ci: add CodSpeed benchmark reporting - #1949

Open
Benoît Cortier (CBenoit) wants to merge 6 commits into
cbenoit-connector-driven-replayfrom
cbenoit-codspeed-benchmark-reporting
Open

Benoît Cortier (CBenoit) wants to merge 6 commits into
cbenoit-connector-driven-replayfrom
cbenoit-codspeed-benchmark-reporting

Conversation

@CBenoit

Copy link
Copy Markdown
Member

Report focused codec and replay workloads separately via CodSpeed while keeping ordinary Criterion and Hyperfine paths unchanged.

Gate service upload until the repository integration and an approved wall-time runner are configured.

Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com

@github-actions github-actions Bot added risk/low Self-contained change with no cross-crate behavioral effect scope/tooling Build, CI, release, or developer tooling size/M Size: up to 449 counted lines and 10 files; exceeds S in either measure labels Sep 10, 2026
@CBenoit
Benoît Cortier (CBenoit) force-pushed the cbenoit-codspeed-benchmark-reporting branch from e69e279 to 81f2e3d Compare September 10, 2026 19:20
@CBenoit
Benoît Cortier (CBenoit) force-pushed the cbenoit-codspeed-benchmark-reporting branch from 81f2e3d to 4c81aad Compare September 10, 2026 20:47
@CBenoit
Benoît Cortier (CBenoit) force-pushed the cbenoit-codspeed-benchmark-reporting branch from 4c81aad to e568f80 Compare September 10, 2026 20:56
@CBenoit
Benoît Cortier (CBenoit) force-pushed the cbenoit-codspeed-benchmark-reporting branch from e568f80 to 2fc678c Compare September 10, 2026 21:02
@CBenoit
Benoît Cortier (CBenoit) force-pushed the cbenoit-codspeed-benchmark-reporting branch from 2fc678c to 227b8aa Compare September 11, 2026 01:06
@CBenoit
Benoît Cortier (CBenoit) added this pull request to stack #1955 September 11, 2026 01:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The CodSpeed workflow has a cache-key inconsistency and warrants human review.

Pull request overview

Adds optional CodSpeed reporting for codec and replay benchmarks while preserving existing Criterion and Hyperfine workflows.

Changes:

  • Adds CodSpeed benchmark integration and configuration.
  • Adds gated GitHub Actions workflows.
  • Documents usage and updates dependencies.
File summaries
File Summary
xtask/README.md Documents CodSpeed workflows and usage.
crates/ironrdp-bench/Cargo.toml Adds optional CodSpeed support.
crates/ironrdp-bench/benches/capture_replay.rs Enables CodSpeed replay benchmarks.
crates/ironrdp-bench/benches/bench.rs Enables CodSpeed codec benchmarks.
codspeed.yml Defines wall-time replay commands.
Cargo.lock Locks CodSpeed dependencies.
.github/workflows/codspeed.yml Adds gated CodSpeed jobs.
Review details

Suppressed comments (1)

.github/workflows/codspeed.yml:43

  • This uses a different cache-key prefix (codspeed-corpus) from the existing corpus cache (benchmark-corpus in .github/workflows/ci.yml:211), so CodSpeed cannot reuse the already populated cache and will redownload the same captures on every cold CodSpeed run. Reuse the existing key if the cache is intended to be shared, as the README states.
          key: ${{ runner.os }}-codspeed-corpus-${{ hashFiles('crates/ironrdp-bench/corpus.toml') }}
  • Files reviewed: 6/7 changed files
  • Comments generated: 0
  • Review effort level: Lite

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #1949 adds CodSpeed benchmark reporting: a gated workflow (simulation on push/PR/dispatch behind CODSPEED_ENABLED; walltime dispatch-only behind a maintainer-set runner variable), a root codspeed.yml with three walltime commands matching the capture-replay-bench CLI, a cfg-gated codspeed feature aliasing codspeed-criterion-compat in both benches, generated Cargo.lock changes, and README docs that accurately describe the gating and OIDC/no-secret posture. Reviewed independently: job gating prevents fork PRs from reaching the walltime runner; prepare/verify run outside timed iterations; corpus use is digest-verified and consistent with existing CI conventions. Only publishable issue is the specialist's valid low-severity Cargo.toml redundancy, accepted unchanged; no other correctness, safety, or maintainability impact found.

Comment thread crates/ironrdp-bench/Cargo.toml Outdated

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR 1949 adds CodSpeed benchmark reporting: a new gated workflow (simulation plus manual-only walltime), a root codspeed.yml walltime manifest, an optional codspeed feature swapping codspeed-criterion-compat for Criterion in the two bench targets, and README documentation. Independent inspection found no correctness, safety, or protocol defects: the feature-gated criterion alias compiles in both configurations, the optional dependency correctly lives in [dependencies] because optional dev-dependencies are unsupported, the walltime job is properly gated against dispatcher-controlled runners, and benchmark identities in docs match the code. Only low-severity maintainability duplication was found, matching the code-compressor candidates, which are accepted.

Comment thread .github/workflows/codspeed.yml
Comment thread xtask/README.md Outdated
Comment thread xtask/README.md Outdated
@github-actions github-actions Bot added ai-reviewed/2 Final automated review completed maintainer-required Maintainer review or intervention is required and removed ai-reviewed/1 One automated review completed labels Sep 11, 2026
Report focused codec and replay workloads separately via CodSpeed while keeping ordinary Criterion and Hyperfine paths unchanged.

Gate service upload until the repository integration and an approved wall-time runner are configured.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Resolve the runner label from a repository-controlled variable so dispatchers cannot select hosted or sensitive self-hosted runners.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep CodSpeed corpus caches outside the dependencies directory so benchmark data remains isolated from native runtime dependencies.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Use the dependency key directly because it already matches the package name.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Reference existing local benchmark commands and state the walltime gates once.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-reviewed/2 Final automated review completed maintainer-required Maintainer review or intervention is required risk/low Self-contained change with no cross-crate behavioral effect scope/tooling Build, CI, release, or developer tooling size/M Size: up to 449 counted lines and 10 files; exceeds S in either measure

Development

Successfully merging this pull request may close these issues.

3 participants