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

Migrate to pyo3 v0.21 Bound API #23

Merged
merged 9 commits into from
Mar 10, 2024
Merged

Migrate to pyo3 v0.21 Bound API #23

merged 9 commits into from
Mar 10, 2024

Conversation

MarshalX
Copy link
Owner

@MarshalX MarshalX commented Mar 3, 2024

they allowed to manage memory from binding side instead of pyo3 internals (gif refs pool). i expect perf boost here

Copy link

codspeed-hq bot commented Mar 3, 2024

CodSpeed Performance Report

Merging #23 will improve performances by 54.73%

Comparing pyo3-0-21 (fa1aadf) with main (8365eab)

Summary

⚡ 6 improvements
✅ 186 untouched benchmarks

Benchmarks breakdown

Benchmark main pyo3-0-21 Change
test_dag_cbor_decode_fixtures[array-6433713753386423] 30.4 µs 26.6 µs +14.19%
test_dag_cbor_decode_fixtures[dagpb_simple_forms_3] 101.1 µs 87.8 µs +15.13%
test_dag_cbor_encode_real_data[canada.json] 73.3 ms 56.2 ms +30.46%
test_dag_cbor_encode_real_data[citm_catalog.json] 118.2 ms 76.4 ms +54.73%
test_dag_cbor_encode_real_data[github.json] 4.2 ms 3.8 ms +11.01%
test_dag_cbor_encode_real_data[twitter.json] 36.9 ms 32 ms +15.34%

@MarshalX
Copy link
Owner Author

MarshalX commented Mar 3, 2024

perf boost in decoding:
image

degradation in encoding. probably i forgot to migrate something:
image

@LilyFoote
Copy link

Looks to me like you missed &PyAny in:

fn encode_dag_cbor_from_pyobject<'py, W: Write>(py: Python<'py>, obj: &'py PyAny, w: &mut W) -> Result<()> {

and
fn encode_multibase(code: char, data: &PyAny) -> PyResult<String> {

@MarshalX
Copy link
Owner Author

MarshalX commented Mar 3, 2024

@LilyFoote thank you! now it much much better! still have 1 regress but ig because of new allocs in sort_map_keys. previously it used refs only. overall perf increased!

decode increased in all cases 🎉🎉:
image

encode:
image
image

src/lib.rs Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
Co-authored-by: David Hewitt <mail@davidhewitt.dev>
@MarshalX
Copy link
Owner Author

MarshalX commented Mar 8, 2024

I found this in PyO3/pyo3#3684

Update py.None(), py.Ellipsis() and py.NotImplemented() to return Borrowed<'py, 'py, PyNone> (etc.)

So should I update py.None() somehow?

@davidhewitt
Copy link

No need; early in 0.21 development I changed those APIs to return GIL Refs e.g. &PyNone. But when the Bound API became the main focus of 0.21 I reverted that change, they just return PyObject again which is fine for now.

@MarshalX
Copy link
Owner Author

MarshalX commented Mar 9, 2024

new record! +52.9%. ive returned back to main branch and added profile-guided optimization:

image

@MarshalX MarshalX changed the title Test pyo3 v0.21-dev Migrate to pyo3 v0.21 Bound API Mar 10, 2024
@MarshalX MarshalX merged commit 8be60ee into main Mar 10, 2024
3 checks passed
@MarshalX MarshalX deleted the pyo3-0-21 branch March 10, 2024 22:29
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