optimize Py<T>::drop for the case when attached (#5454)

* optimize `Py<T>::drop` for the case when attached

* newsfragment

Author: davidhewitt

newsfragments/5454.changed.md

@@ -0,0 +1 @@
+Optimize `Py<T>::drop` for the case when attached to the Python interpreter.

src/instance.rs

@@ -2020,10 +2020,29 @@ where
 #[cfg(feature = "py-clone")]
 impl<T> Clone for Py<T> {
     #[track_caller]
+    #[inline]
     fn clone(&self) -> Self {
-        unsafe {
-            state::register_incref(self.0);
+        #[track_caller]
+        #[inline]
+        fn try_incref(obj: NonNull<ffi::PyObject>) {
+            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)
     }
 }
@@ -2037,11 +2056,30 @@ impl<T> Clone for Py<T> {
 /// However, if the `pyo3_disable_reference_pool` conditional compilation flag
 /// is enabled, it will abort the process.
 impl<T> Drop for Py<T> {
-    #[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<ffi::PyObject>) {
+            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<ffi::PyObject>) {
+            // SAFETY: handing ownership of the reference to `register_decref`.
+            unsafe {
+                state::register_decref(obj);
+            }
+        }
+
+        inner(self.0)
     }
 }
 

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,44 +298,28 @@ 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<ffi::PyObject>) {
-    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.
 ///
 /// If the thread is attached, the reference count will be decreased immediately instead of being queued
 /// 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<ffi::PyObject>) {
-    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.");
     }
 }