From 00a459d9bca66da444c7ff5429c9ae7adbbbed6f Mon Sep 17 00:00:00 2001 From: Juniper Tyree <50025784+juntyr@users.noreply.github.com> Date: Mon, 19 Feb 2024 08:53:55 +0000 Subject: [PATCH 1/7] add Py::drop_ref method --- src/instance.rs | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/instance.rs b/src/instance.rs index b83b69a20d3..cbd3dc865fb 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -817,6 +817,10 @@ impl IntoPy for Borrowed<'_, '_, T> { /// Otherwise, the reference count will be decreased the next time the GIL is /// reacquired. /// +/// If you happen to be already holding the GIL, [`Py::drop_ref`] will decrease +/// the Python reference count immediately and will execute slightly faster than +/// relying on implicit [`Drop`]s. +/// /// # A note on `Send` and `Sync` /// /// Accessing this object is threadsafe, since any access to its API requires a [`Python<'py>`](crate::Python) token. @@ -1215,6 +1219,35 @@ impl Py { unsafe { Py::from_borrowed_ptr(py, self.0.as_ptr()) } } + /// Drops `self` and immediately decreases its reference count. + /// + /// This method is a micro-optimisation over [`Drop`] if you happen to be holding the GIL + /// already. + /// + /// # Examples + /// + /// ```rust + /// use pyo3::prelude::*; + /// use pyo3::types::PyDict; + /// + /// # fn main() { + /// Python::with_gil(|py| { + /// let object: Py = PyDict::new_bound(py).unbind(); + /// + /// // some usage of object + /// + /// object.drop_ref(py); + /// }); + /// # } + /// ``` + #[inline] + pub fn drop_ref(self, py: Python<'_>) { + let _py = py; + + // Safety: we hold the GIL and forget `self` to not trigger a double free + unsafe { pyo3::ffi::Py_DECREF(self.into_ptr()) }; + } + /// Returns whether the object is considered to be None. /// /// This is equivalent to the Python expression `self is None`. From 9d23eb7f7026fd2b46dfc5ccf4cad684f0bb0555 Mon Sep 17 00:00:00 2001 From: Juniper Tyree <50025784+juntyr@users.noreply.github.com> Date: Mon, 19 Feb 2024 08:58:25 +0000 Subject: [PATCH 2/7] add changelog entry --- newsfragments/3871.added.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/3871.added.md diff --git a/newsfragments/3871.added.md b/newsfragments/3871.added.md new file mode 100644 index 00000000000..f90e92fdfff --- /dev/null +++ b/newsfragments/3871.added.md @@ -0,0 +1 @@ +Add `Py::drop_ref` to explicitly drop a `Py`` and immediately decrease the Python reference count if the GIL is already held. From 06853096ad75a567715ef82c79a90bf44c7ce478 Mon Sep 17 00:00:00 2001 From: Juniper Tyree <50025784+juntyr@users.noreply.github.com> Date: Mon, 19 Feb 2024 09:00:46 +0000 Subject: [PATCH 3/7] fix ffi import --- src/instance.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/instance.rs b/src/instance.rs index cbd3dc865fb..6f5c76583bc 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -1245,7 +1245,7 @@ impl Py { let _py = py; // Safety: we hold the GIL and forget `self` to not trigger a double free - unsafe { pyo3::ffi::Py_DECREF(self.into_ptr()) }; + unsafe { ffi::Py_DECREF(self.into_ptr()) }; } /// Returns whether the object is considered to be None. From c7f91ef2ff781c527a4023dea7032ad504bc6a85 Mon Sep 17 00:00:00 2001 From: Juniper Tyree <50025784+juntyr@users.noreply.github.com> Date: Mon, 19 Feb 2024 09:13:21 +0000 Subject: [PATCH 4/7] integrate review feedback --- src/instance.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/instance.rs b/src/instance.rs index 6f5c76583bc..af812b48da2 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -1224,6 +1224,9 @@ impl Py { /// This method is a micro-optimisation over [`Drop`] if you happen to be holding the GIL /// already. /// + /// Note that if you are using [`Bound`], you do not need to use [`Self::drop_ref`] since + /// [`Bound`] guarantees that the GIL is held. + /// /// # Examples /// /// ```rust @@ -1242,10 +1245,7 @@ impl Py { /// ``` #[inline] pub fn drop_ref(self, py: Python<'_>) { - let _py = py; - - // Safety: we hold the GIL and forget `self` to not trigger a double free - unsafe { ffi::Py_DECREF(self.into_ptr()) }; + let _ = self.into_bound(py); } /// Returns whether the object is considered to be None. From 00f2b179a17ec988e58f7d9baaa88be4be71db65 Mon Sep 17 00:00:00 2001 From: Juniper Tyree <50025784+juntyr@users.noreply.github.com> Date: Wed, 21 Feb 2024 16:29:18 +0000 Subject: [PATCH 5/7] Add a test --- src/instance.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/instance.rs b/src/instance.rs index af812b48da2..bc523d7f95f 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -2162,6 +2162,22 @@ a = A() }) } + #[test] + fn explicit_drop_ref() { + Python::with_gil(|py| { + let object: Py = PyDict::new_bound(py).unbind(); + let object2 = object.clone_ref(py); + + assert_eq!(object.get_refcnt(), 2); + + object.drop_ref(py); + + assert_eq!(object.get_refcnt(), 1); + + object2.drop_ref(py); + }); + } + #[cfg(feature = "macros")] mod using_macros { use crate::PyCell; From 8de1b13cf50a83a197d2f2ba64b565144855223b Mon Sep 17 00:00:00 2001 From: Juniper Tyree <50025784+juntyr@users.noreply.github.com> Date: Wed, 21 Feb 2024 16:32:05 +0000 Subject: [PATCH 6/7] Fix some build errors --- src/instance.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/instance.rs b/src/instance.rs index bc523d7f95f..7ea3bd505d3 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -2168,11 +2168,11 @@ a = A() let object: Py = PyDict::new_bound(py).unbind(); let object2 = object.clone_ref(py); - assert_eq!(object.get_refcnt(), 2); + assert_eq!(object.get_refcnt(py), 2); object.drop_ref(py); - assert_eq!(object.get_refcnt(), 1); + assert_eq!(object.get_refcnt(py), 1); object2.drop_ref(py); }); From 5f3506981bf80dfff377a667fceee0fb2fa9d31d Mon Sep 17 00:00:00 2001 From: Juniper Tyree <50025784+juntyr@users.noreply.github.com> Date: Wed, 21 Feb 2024 16:36:08 +0000 Subject: [PATCH 7/7] Fix some more build errors --- src/instance.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/instance.rs b/src/instance.rs index 7ea3bd505d3..e88cefd2313 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -2168,11 +2168,12 @@ a = A() let object: Py = PyDict::new_bound(py).unbind(); let object2 = object.clone_ref(py); + assert_eq!(object.as_ptr(), object2.as_ptr()); assert_eq!(object.get_refcnt(py), 2); object.drop_ref(py); - assert_eq!(object.get_refcnt(py), 1); + assert_eq!(object2.get_refcnt(py), 1); object2.drop_ref(py); });