From 0100a9fbde6e7b4ffad7e616d82efba93f5be897 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Fri, 19 Sep 2025 15:52:23 +0100 Subject: [PATCH 1/2] optimize `Py::drop` for the case when attached --- src/instance.rs | 48 ++++++++++++++++++++++++++++++++++++++----- src/internal/state.rs | 44 +++++++++++++-------------------------- 2 files changed, 57 insertions(+), 35 deletions(-) diff --git a/src/instance.rs b/src/instance.rs index 3fc37cd0d41..289a0adfdf5 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -2009,10 +2009,29 @@ where #[cfg(feature = "py-clone")] impl Clone for Py { #[track_caller] + #[inline] fn clone(&self) -> Self { - unsafe { - state::register_incref(self.0); + #[track_caller] + #[inline] + fn try_incref(obj: NonNull) { + use crate::internal::state::thread_is_attached; + + if thread_is_attached() { + // SAFETY: Py_INCREF is safe to call on a valid Python object if the thread is attached. + unsafe { ffi::Py_INCREF(obj.as_ptr()) } + } else { + incref_failed() + } } + + #[cold] + #[track_caller] + fn incref_failed() -> ! { + panic!("Cannot clone pointer into Python heap without the thread being attached."); + } + + try_incref(self.0); + Self(self.0, PhantomData) } } @@ -2026,11 +2045,30 @@ impl Clone for Py { /// However, if the `pyo3_disable_reference_pool` conditional compilation flag /// is enabled, it will abort the process. impl Drop for Py { - #[track_caller] + #[inline] fn drop(&mut self) { - unsafe { - state::register_decref(self.0); + // non generic inlineable inner function to reduce code bloat + #[inline] + fn inner(obj: NonNull) { + use crate::internal::state::thread_is_attached; + + if thread_is_attached() { + // SAFETY: Py_DECREF is safe to call on a valid Python object if the thread is attached. + unsafe { ffi::Py_DECREF(obj.as_ptr()) } + } else { + drop_slow(obj) + } } + + #[cold] + fn drop_slow(obj: NonNull) { + // SAFETY: handing ownership of the reference to `register_decref`. + unsafe { + state::register_decref(obj); + } + } + + inner(self.0) } } diff --git a/src/internal/state.rs b/src/internal/state.rs index 4105b31792f..899c37bd13e 100644 --- a/src/internal/state.rs +++ b/src/internal/state.rs @@ -32,7 +32,7 @@ const ATTACH_FORBIDDEN_DURING_TRAVERSE: isize = -1; /// 2) PyGILState_Check always returns 1 if the sub-interpreter APIs have ever been called, /// which could lead to incorrect conclusions that the thread is attached. #[inline(always)] -fn thread_is_attached() -> bool { +pub(crate) fn thread_is_attached() -> bool { ATTACH_COUNT.try_with(|c| c.get() > 0).unwrap_or(false) } @@ -298,21 +298,6 @@ impl Drop for ForbidAttaching { } } -/// Increments the reference count of a Python object if the thread is attached. If -/// the thread is not attached, this function will panic. -/// -/// # Safety -/// The object must be an owned Python reference. -#[cfg(feature = "py-clone")] -#[track_caller] -pub unsafe fn register_incref(obj: NonNull) { - if thread_is_attached() { - unsafe { ffi::Py_INCREF(obj.as_ptr()) } - } else { - panic!("Cannot clone pointer into Python heap without the thread being attached."); - } -} - /// Registers a Python object pointer inside the release pool, to have its reference count decreased /// the next time the thread is attached in pyo3. /// @@ -320,22 +305,21 @@ pub unsafe fn register_incref(obj: NonNull) { /// for later. /// /// # Safety -/// The object must be an owned Python reference. -#[track_caller] +/// - The object must be an owned Python reference. +/// - The reference must not be used after calling this function. +#[inline] pub unsafe fn register_decref(obj: NonNull) { - if thread_is_attached() { - unsafe { ffi::Py_DECREF(obj.as_ptr()) } - } else { - #[cfg(not(pyo3_disable_reference_pool))] + #[cfg(not(pyo3_disable_reference_pool))] + { get_pool().register_decref(obj); - #[cfg(all( - pyo3_disable_reference_pool, - not(pyo3_leak_on_drop_without_reference_pool) - ))] - { - let _trap = PanicTrap::new("Aborting the process to avoid panic-from-drop."); - panic!("Cannot drop pointer into Python heap without the thread being attached."); - } + } + #[cfg(all( + pyo3_disable_reference_pool, + not(pyo3_leak_on_drop_without_reference_pool) + ))] + { + let _trap = PanicTrap::new("Aborting the process to avoid panic-from-drop."); + panic!("Cannot drop pointer into Python heap without the thread being attached."); } } From 362c77db86f5535ee49b5e6f5ee0f6f683caf6ea Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Fri, 19 Sep 2025 15:55:40 +0100 Subject: [PATCH 2/2] newsfragment --- newsfragments/5454.changed.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/5454.changed.md diff --git a/newsfragments/5454.changed.md b/newsfragments/5454.changed.md new file mode 100644 index 00000000000..e65a6e2ae47 --- /dev/null +++ b/newsfragments/5454.changed.md @@ -0,0 +1 @@ +Optimize `Py::drop` for the case when attached to the Python interpreter.