Skip to content

Restore pytest-benchmark for local and manual profiling - #1113

Open
Siyet wants to merge 1 commit into
mainfrom
fix-profile-workflow-benchmark-flags
Open

Restore pytest-benchmark for local and manual profiling#1113
Siyet wants to merge 1 commit into
mainfrom
fix-profile-workflow-benchmark-flags

Conversation

@Siyet

@Siyet Siyet commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

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:

pytest: error: unrecognized arguments: --benchmark-name --benchmark-disable-gc

(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):

  • just test-perf all ... (no --codspeed): pytest-benchmark provides the benchmark fixture and prints its walltime table. The --benchmark-* / --calibrate / --rounds flags are recognized again.
  • codspeed.yml (--codspeed): pytest-codspeed instruments the same fixture; pytest-benchmark steps aside. No conflict.

uv.lock is gitignored, so the diff is just the dependency line; CI resolves the env from pyproject.toml.

@Siyet
Siyet temporarily deployed to docs-preview July 3, 2026 11:56 — with GitHub Actions Inactive
provinzkraut
provinzkraut previously approved these changes Jul 3, 2026

@ofek ofek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

@Siyet
Siyet force-pushed the fix-profile-workflow-benchmark-flags branch from f40e1b6 to 513cd64 Compare July 4, 2026 11:48
@Siyet Siyet changed the title Fix Profile workflow by dropping stale pytest-benchmark flags Restore pytest-benchmark for local and manual profiling Jul 4, 2026
@Siyet

Siyet commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

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

@codspeed-hq

codspeed-hq Bot commented Jul 4, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 11.26%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 3 improved benchmarks
✅ 136 untouched benchmarks
⏩ 135 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation test_decode_type[arm-msgpack-True] 824.3 µs 735.6 µs +12.06%
Simulation test_decode_type[arm-json-None] 872.8 µs 782.2 µs +11.58%
Simulation test_decode_type[arm-json-False] 960.9 µs 872.4 µs +10.14%

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)

Open in CodSpeed

Footnotes

  1. 135 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@ofek

ofek commented Jul 4, 2026

Copy link
Copy Markdown
Member

I'll take on the work to bring them back, thanks!

@Siyet

Siyet commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants