Use GILProtected for capsule caching? #263
Comments
|
This is probably a question for @gracinet. |
SimonSapin
added a commit
to SimonSapin/rust-cpython
that referenced
this issue
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`. `RefCell` might work too. Fixes dgrunwald#263
|
I’ve submitted #268. |
SimonSapin
added a commit
to SimonSapin/rust-cpython
that referenced
this issue
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 dgrunwald#263
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The
py_capsuleandpy_capsule_fnmacros generate code that cache the pointer obtained from a capsule in astatic mutitem, with astd::sync::Oncein a secondstaticitem for thread-safety.Although the synchronization cost of
Onceis small, it seems unnecessary here since code touching this is always holding the GIL. Could this cache be implemented with a singlestaticitem of typecpython::GILProtected<std::cell::Cell<_>>instead?The text was updated successfully, but these errors were encountered: