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

Cheaper and safe caching for py_capsule! and py_capsule_fn! #268

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SimonSapin
Copy link
Contributor

@SimonSapin SimonSapin commented Aug 12, 2021

The py_capsule! and py_capsule_fn! macros generate retrieve functions that cache their results for subsequent calls.

Prior to this commit, caching is done with a generated unsafe static mut item whose access is made thread-safe by a std::sync::Once on the side. However this synchronization is unnecessary since retreive takes a Python<'_> parameter that indicates that the GIL is held.

This changes the cache to use safe static item instead, with GILProtected for synchronization-free thread-safety and OnceCell for interior mutability. As a bonus, this removes the need for cache-related unsafe code in generated retrieve functions.

This adds a dependency to the once_cell crate, which can be removed when OnceCell becomes stable in the standard library: rust-lang/rust#74465

Alternatively OnceCell could be replaced with UnsafeCell plus some unsafe code, effectively inlining the core of the logic of OnceCell.

Fixes #263

The `py_capsule!` and `py_capsule_fn!` macros generate `retrieve` functions
that cache their results for subsequent calls.

Prior to this commit, caching is done with a generated unsafe `static mut`
item whose access is made thread-safe by a `std::sync::Once` on the side.
However this synchronization is unnecessary since `retreive` takes a
`Python<'_>` parameter that indicates that the GIL is held.

This changes the cache to use safe `static` item instead, with `GILProtected`
for synchronization-free thread-safety and `OnceCell` for interior mutability.
As a bonus, this removes the need for cache-related `unsafe` code in
generated `retrieve` functions.

This adds a dependency to the `once_cell` crate, which can be removed when
`OnceCell` becomes stable in the standard library:
rust-lang/rust#74465

Alternatively `OnceCell` could be replaced with `UnsafeCell` plus some
`unsafe` code, effectively inlining the core of the logic of `OnceCell`.

Fixes dgrunwald#263
@SimonSapin
Copy link
Contributor Author

@markbt, any thought on this?

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.

Use GILProtected for capsule caching?
1 participant