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

pyclass methods for Bound #3792

Merged
merged 1 commit into from
Feb 5, 2024
Merged

Conversation

davidhewitt
Copy link
Member

Split from #3606

This PR adds new methods to Bound<T: PyClass> to access the inner T via PyRef<T>, PyRefMut<T> and &T.

These methods are identical to the ones on Py<T>.

Given that we want to remove the GIL Ref &PyCell<T>, I think it is correct to add these methods to Bound<T> as a replacement.

@davidhewitt davidhewitt added the CI-skip-changelog Skip checking changelog entry label Feb 3, 2024
@adamreichold
Copy link
Member

Is there something we'd want to conditionally deprecate here? I don't think the corresponding methods on PyCell are reasonable, but some way to produce a &PyCell? But I guess this will usually happen behind the scenes a function/method arguments?

@davidhewitt
Copy link
Member Author

Yes I think that any associated deprecation would be the same one that deprecates getting GIL Refs in general, if we can find a nice pattern for that. Probably by deprecating .as_gil_ref() and .into_gil_ref() methods plus finding a mechanism to deprecate the function arguments in macro code.

@davidhewitt
Copy link
Member Author

(Or maybe we deprecate PyCell completely, #3792 (comment). Probably too much to do just in this PR, however.)

@davidhewitt
Copy link
Member Author

Thanks for the approval! I'll push some coverage and merge 👍

Copy link

codspeed-hq bot commented Feb 5, 2024

CodSpeed Performance Report

Merging #3792 will improve performances by 20.41%

Comparing davidhewitt:bound-pyclass-2 (42843de) with main (7938d4c)

🎉 Hooray! codspeed-rust just leveled up to 2.3.3!

A heads-up, this is a breaking change and it might affect your current performance baseline a bit. But here's the exciting part - it's packed with new, cool features and promises improved result stability 🥳!
Curious about what's new? Visit our releases page to delve into all the awesome details about this new version.

Summary

⚡ 3 improvements
✅ 76 untouched benchmarks

Benchmarks breakdown

Benchmark main davidhewitt:bound-pyclass-2 Change
not_a_list_via_downcast 242.8 ns 215 ns +12.92%
mapping_from_dict 327.8 ns 272.2 ns +20.41%
sequence_from_list 355.6 ns 300 ns +18.52%

@davidhewitt davidhewitt added this pull request to the merge queue Feb 5, 2024
Merged via the queue into PyO3:main with commit 02f1df6 Feb 5, 2024
36 checks passed
@davidhewitt davidhewitt deleted the bound-pyclass-2 branch February 5, 2024 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI-skip-changelog Skip checking changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants