From 3bebc19e2dc4834e1deb3d4fd3ee6a9f6d27660d Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Sun, 6 Jun 2021 07:48:21 +0200 Subject: [PATCH] PyList: remove get_parked_item, use macros for speed on !abi3 --- CHANGELOG.md | 1 + src/types/dict.rs | 2 +- src/types/list.rs | 82 ++++++++++++++++++----------------------------- 3 files changed, 33 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 78c3de06..8825b1ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Remove `__doc__` from module's `__all__`. [#1509](https://github.com/PyO3/pyo3/pull/1509) - Remove `PYO3_CROSS_INCLUDE_DIR` environment variable and the associated C header parsing functionality. [#1521](https://github.com/PyO3/pyo3/pull/1521) - Remove `raw_pycfunction!` macro. [#1619](https://github.com/PyO3/pyo3/pull/1619) +- Remove `PyList::get_parked_item`. [#1664](https://github.com/PyO3/pyo3/pull/1664) ### Fixed - Remove FFI definition `PyCFunction_ClearFreeList` for Python 3.9 and later. [#1425](https://github.com/PyO3/pyo3/pull/1425) diff --git a/src/types/dict.rs b/src/types/dict.rs index 5d912e37..e58afccb 100644 --- a/src/types/dict.rs +++ b/src/types/dict.rs @@ -51,7 +51,7 @@ impl PyDict { /// Returns a new dictionary that contains the same key-value pairs as self. /// - /// This is equivalent to the Python expression `dict(self)`. + /// This is equivalent to the Python expression `self.copy()`. pub fn copy(&self) -> PyResult<&PyDict> { unsafe { self.py() diff --git a/src/types/list.rs b/src/types/list.rs index 8692688d..7649d4b2 100644 --- a/src/types/list.rs +++ b/src/types/list.rs @@ -15,6 +15,22 @@ pub struct PyList(PyAny); pyobject_native_type_core!(PyList, ffi::PyList_Type, #checkfunction=ffi::PyList_Check); +#[inline] +unsafe fn new_from_iter( + elements: impl ExactSizeIterator, + convert: impl Fn(T) -> PyObject, +) -> *mut ffi::PyObject { + let ptr = ffi::PyList_New(elements.len() as Py_ssize_t); + for (i, e) in elements.enumerate() { + let obj = convert(e).into_ptr(); + #[cfg(not(Py_LIMITED_API))] + ffi::PyList_SET_ITEM(ptr, i as Py_ssize_t, obj); + #[cfg(Py_LIMITED_API)] + ffi::PyList_SetItem(ptr, i as Py_ssize_t, obj); + } + ptr +} + impl PyList { /// Constructs a new list with the given elements. pub fn new(py: Python<'_>, elements: impl IntoIterator) -> &PyList @@ -22,15 +38,8 @@ impl PyList { T: ToPyObject, U: ExactSizeIterator, { - let elements_iter = elements.into_iter(); - let len = elements_iter.len(); unsafe { - let ptr = ffi::PyList_New(len as Py_ssize_t); - for (i, e) in elements_iter.enumerate() { - let obj = e.to_object(py).into_ptr(); - ffi::PyList_SetItem(ptr, i as Py_ssize_t, obj); - } - py.from_owned_ptr::(ptr) + py.from_owned_ptr::(new_from_iter(elements.into_iter(), |e| e.to_object(py))) } } @@ -41,8 +50,15 @@ impl PyList { /// Returns the length of the list. pub fn len(&self) -> usize { - // non-negative Py_ssize_t should always fit into Rust usize - unsafe { ffi::PyList_Size(self.as_ptr()) as usize } + unsafe { + #[cfg(not(Py_LIMITED_API))] + let size = ffi::PyList_GET_SIZE(self.as_ptr()); + #[cfg(Py_LIMITED_API)] + let size = ffi::PyList_Size(self.as_ptr()); + + // non-negative Py_ssize_t should always fit into Rust usize + size as usize + } } /// Checks if the list is empty. @@ -56,6 +72,9 @@ impl PyList { pub fn get_item(&self, index: isize) -> &PyAny { assert!((index.abs() as usize) < self.len()); unsafe { + #[cfg(not(Py_LIMITED_API))] + let ptr = ffi::PyList_GET_ITEM(self.as_ptr(), index as Py_ssize_t); + #[cfg(Py_LIMITED_API)] let ptr = ffi::PyList_GetItem(self.as_ptr(), index as Py_ssize_t); // PyList_GetItem return borrowed ptr; must make owned for safety (see #890). @@ -64,18 +83,6 @@ impl PyList { } } - /// Gets the item at the specified index. - /// - /// Panics if the index is out of range. - pub fn get_parked_item(&self, index: isize) -> PyObject { - unsafe { - PyObject::from_borrowed_ptr( - self.py(), - ffi::PyList_GetItem(self.as_ptr(), index as Py_ssize_t), - ) - } - } - /// Sets the item at the specified index. /// /// Panics if the index is out of range. @@ -167,14 +174,7 @@ where T: ToPyObject, { fn to_object(&self, py: Python<'_>) -> PyObject { - unsafe { - let ptr = ffi::PyList_New(self.len() as Py_ssize_t); - for (i, e) in self.iter().enumerate() { - let obj = e.to_object(py).into_ptr(); - ffi::PyList_SetItem(ptr, i as Py_ssize_t, obj); - } - PyObject::from_owned_ptr(py, ptr) - } + unsafe { PyObject::from_owned_ptr(py, new_from_iter(self.iter(), |e| e.to_object(py))) } } } @@ -192,14 +192,7 @@ where T: IntoPy, { fn into_py(self, py: Python) -> PyObject { - unsafe { - let ptr = ffi::PyList_New(self.len() as Py_ssize_t); - for (i, e) in self.into_iter().enumerate() { - let obj = e.into_py(py).into_ptr(); - ffi::PyList_SetItem(ptr, i as Py_ssize_t, obj); - } - PyObject::from_owned_ptr(py, ptr) - } + unsafe { PyObject::from_owned_ptr(py, new_from_iter(self.into_iter(), |e| e.into_py(py))) } } } @@ -244,19 +237,6 @@ mod test { assert_eq!(7, list.get_item(3).extract::().unwrap()); } - #[test] - fn test_get_parked_item() { - let gil = Python::acquire_gil(); - let py = gil.python(); - let v = vec![2, 3, 5, 7]; - let ob = v.to_object(py); - let list = ::try_from(ob.as_ref(py)).unwrap(); - assert_eq!(2, list.get_parked_item(0).extract::(py).unwrap()); - assert_eq!(3, list.get_parked_item(1).extract::(py).unwrap()); - assert_eq!(5, list.get_parked_item(2).extract::(py).unwrap()); - assert_eq!(7, list.get_parked_item(3).extract::(py).unwrap()); - } - #[test] fn test_set_item() { let gil = Python::acquire_gil();