Skip to content

Add codspeed benchmarking in a dedicated CI job - #1427

Merged
sirosen merged 1 commit into
mainfrom
add-codspeed-benchmarking
Sep 8, 2026
Merged

Add codspeed benchmarking in a dedicated CI job#1427
sirosen merged 1 commit into
mainfrom
add-codspeed-benchmarking

Conversation

@sirosen

@sirosen sirosen commented Sep 4, 2026

Copy link
Copy Markdown
Member

Also add a new tox config to support running codspeed locally and in CI.

@sirosen
sirosen requested a review from aaschaer as a code owner September 4, 2026 19:43
@sirosen sirosen added the no-news-is-good-news This change does not require a news file label Sep 4, 2026
@codspeed-hq

codspeed-hq Bot commented Sep 4, 2026

Copy link
Copy Markdown

@sirosen

This comment was marked as outdated.

Also add a new tox config to support running codspeed locally and in CI.

@derek-globus derek-globus 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.

First blush with CodSpeed, I'd like to understand better how you expect this tool should fit into the sdk ecosystem.

@@ -0,0 +1,45 @@
name: CodSpeed Benchmarks

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.

Does this just generate an artifact that we can inspect to more easily identify where a performance impact occurred? Or will this do something more aggressive like blocking PRs that hurt performance against prior benchmarks?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It doesn't generate profiles or traces, but it does some sophisticated emulation work to produce stable benchmark results, which it then tracks in a project history.

It can comment on PRs which have significant performance impacts. And it defaults to blocking PRs which show a regression but I have explicitly turned that off for us. Here's how I configured it for now:

image

So if we have a perf regression it will tell us.


For a sample of what the PR comment looks like, I was experimenting with this in slyp over the weekend a little:
sirosen/slyp#34

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.

Okay, thanks; that's helpful

@sirosen
sirosen force-pushed the add-codspeed-benchmarking branch from e8b3625 to bb013a7 Compare September 8, 2026 14:17
@sirosen

sirosen commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

FWIW, I'm also new to codspeed. I'd like us to explore it, as a harness for running and tracking benchmark history, and improving our org-wide discipline. If it doesn't work out, we can remove it.

@sirosen

sirosen commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

I fixed the CI failure from codspeed; I was previously wrong about the cause. The tox env needs passenv = CODSPEED_* so that codspeed can pass info via those environment variables.

@derek-globus derek-globus 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.

I'm onboard to give it a try. If it becomes onerous, I may advocate that we rethink our usage, but I can see the benefit to catching performance impacts before they make it to source code.

@sirosen

sirosen commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

FWIW, I see the value more as giving us a better way of talking about performance impacts. If I want to refactor something to be 3x larger but 10x faster, I should be able to prove that to my reviewers with a new benchmark. And we want to encode the idea that "this code is longer and more complex than other solutions because it's faster" into our testing process.

@sirosen
sirosen merged commit d32735a into main Sep 8, 2026
12 checks passed
@sirosen
sirosen deleted the add-codspeed-benchmarking branch September 8, 2026 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-news-is-good-news This change does not require a news file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants