Restore pytest-benchmark for local and manual profiling - #1113
Conversation
Sorry, something went wrong.
ofek
left a comment
There was a problem hiding this comment.
Instead, can you please bring back support for the old, simpler benchmarks so that we can run them locally and with the manual CI trigger? I was going to myself actually so I would feel bad merging this and then adding much of this again shortly after.
Sorry, something went wrong.
|
@ofek good call, reworked to your suggestion. Instead of dropping the pytest-benchmark scaffolding, this just re-adds pytest-benchmark to test-prof. It was removed in #1075, which is what orphaned the --benchmark-* flags and crashed the workflow. That restores the simple benchmarks for local runs and the manual profile trigger. I confirmed the two plugins coexist: without --codspeed, pytest-benchmark runs the benchmark fixture (walltime table); with --codspeed, codspeed instruments it and pytest-benchmark steps aside. One thing still open from the old setup: the --calibrate/--rounds conftest options are accepted but no longer wired to anything (the bench/bench_config fixtures that consumed them were also removed in #1075, the tests now call benchmark.pedantic(...) directly). This PR doesn't touch that. If you want those knobs functional again it's a bit more surgery on the tests, happy to leave it to you since you mentioned wanting to take this, or fold it in here. Your call. No rush on my end, just let me know what you think whenever you get a chance. |
Sorry, something went wrong.
Merging this PR will improve performance by 11.26%⚠️ Different runtime environments detected
⚡ 3 improved benchmarks Performance Changes
Tip Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent. Comparing fix-profile-workflow-benchmark-flags (513cd64) with main (9ddc0e9) Footnotes
|
Sorry, something went wrong.
|
I'll take on the work to bring them back, thanks! |
Sorry, something went wrong.
|
Whatever's easiest for you: feel free to merge this and start on yours, or close it and just repeat the one line in your own PR since it's the only change here. I'll leave the fate of this one in your hands. |
Sorry, something went wrong.
Reviewers
Requested changes must be addressed to merge this pull request.
The Profile / Manual Profile workflow crashes on every run: just test-perf appends --benchmark-name short --benchmark-disable-gc, but those are pytest-benchmark options and pytest-benchmark was dropped from test-prof when the perf suite moved to pytest-codspeed (#1075). So every matrix job dies at collection:
(all 5 OS/arch jobs, e.g. run 27883637649).
Rather than strip the pytest-benchmark scaffolding out, this restores it: pytest-benchmark is added back to the test-prof group. That fixes the crash and brings back the simple, locally-runnable benchmarks for just test-perf and the manual profile trigger, while leaving continuous benchmarking on pytest-codspeed untouched.
The two plugins coexist cleanly (verified locally, both installed):
uv.lock is gitignored, so the diff is just the dependency line; CI resolves the env from pyproject.toml.