Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wip test PyO3 HEAD #1450

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

wip test PyO3 HEAD #1450

wants to merge 4 commits into from

Conversation

davidhewitt
Copy link
Contributor

PyO3 head contains some breaking changes and perf tweaks; I want to get early visibility onto those.

Copy link

codspeed-hq bot commented Sep 13, 2024

CodSpeed Performance Report

Merging #1450 will degrade performances by 25.18%

Comparing dh/pyo3-0.23 (5e6b595) with main (6472887)

Summary

❌ 4 regressions
✅ 151 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main dh/pyo3-0.23 Change
test_enum_int_core 15.2 µs 20.2 µs -24.49%
test_enum_str_core 14.8 µs 19.7 µs -25.18%
test_json_any_list_int 195.3 µs 222.7 µs -12.33%
test_to_json_list_of_lists 1.7 ms 2 ms -15.16%

@davidhewitt
Copy link
Contributor Author

Performance diffs do not seem concerning to me; clicking through to codspeed there is a mix of positive and negative changes to about the same scale. Serialization was probably impacted slightly negatively due to the addition of the mutexes here.

PyO3 0.23 is a fairly big change due to freethreaded Python support so the fact there's no particularly big perf change here is a success in my eyes; we believe that future PyO3 versions will be able to optimize following the big changes of 0.23.

@davidhewitt
Copy link
Contributor Author

This is getting big enough that I'm going to halt here; I'm just doing mechanical changes so I'm convinced the upgrade is ok. Will continue it after PyO3 0.23 actually released.

When we merge, will probably do it in phases to make it reviewable.

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.

1 participant