From 248230b8e4d597e15f82f926c04bc71c41e9e229 Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Sun, 15 Jan 2023 10:06:45 +0000 Subject: [PATCH] refactor PyAny::is_instance_of for performance --- benches/bench_any.rs | 35 +++++++++++++++++------------------ guide/src/exception.md | 6 +++--- newsfragments/2881.changed.md | 1 + src/type_object.rs | 3 +++ src/types/any.rs | 13 ++++++++----- src/types/bytes.rs | 4 ++-- src/types/mod.rs | 1 + src/types/sequence.rs | 2 +- tests/test_datetime.rs | 18 +++++++++--------- tests/test_inheritance.rs | 4 ++-- 10 files changed, 47 insertions(+), 40 deletions(-) create mode 100644 newsfragments/2881.changed.md diff --git a/benches/bench_any.rs b/benches/bench_any.rs index b6af45df..ec23cedc 100644 --- a/benches/bench_any.rs +++ b/benches/bench_any.rs @@ -5,7 +5,7 @@ use pyo3::{ PyBool, PyByteArray, PyBytes, PyDict, PyFloat, PyFrozenSet, PyInt, PyList, PyMapping, PySequence, PySet, PyString, PyTuple, }, - PyAny, PyResult, Python, + PyAny, Python, }; #[derive(PartialEq, Eq, Debug)] @@ -27,30 +27,30 @@ enum ObjectType { Unknown, } -fn find_object_type(obj: &PyAny) -> PyResult { - let obj_type = if obj.is_none() { +fn find_object_type(obj: &PyAny) -> ObjectType { + if obj.is_none() { ObjectType::None - } else if obj.is_instance_of::()? { + } else if obj.is_instance_of::() { ObjectType::Bool - } else if obj.is_instance_of::()? { + } else if obj.is_instance_of::() { ObjectType::ByteArray - } else if obj.is_instance_of::()? { + } else if obj.is_instance_of::() { ObjectType::Bytes - } else if obj.is_instance_of::()? { + } else if obj.is_instance_of::() { ObjectType::Dict - } else if obj.is_instance_of::()? { + } else if obj.is_instance_of::() { ObjectType::Float - } else if obj.is_instance_of::()? { + } else if obj.is_instance_of::() { ObjectType::FrozenSet - } else if obj.is_instance_of::()? { + } else if obj.is_instance_of::() { ObjectType::Int - } else if obj.is_instance_of::()? { + } else if obj.is_instance_of::() { ObjectType::List - } else if obj.is_instance_of::()? { + } else if obj.is_instance_of::() { ObjectType::Set - } else if obj.is_instance_of::()? { + } else if obj.is_instance_of::() { ObjectType::Str - } else if obj.is_instance_of::()? { + } else if obj.is_instance_of::() { ObjectType::Tuple } else if obj.downcast::().is_ok() { ObjectType::Sequence @@ -58,17 +58,16 @@ fn find_object_type(obj: &PyAny) -> PyResult { ObjectType::Mapping } else { ObjectType::Unknown - }; - Ok(obj_type) + } } fn bench_identify_object_type(b: &mut Bencher<'_>) { Python::with_gil(|py| { let obj = py.eval("object()", None, None).unwrap(); - b.iter(|| find_object_type(obj).unwrap()); + b.iter(|| find_object_type(obj)); - assert_eq!(find_object_type(obj).unwrap(), ObjectType::Unknown); + assert_eq!(find_object_type(obj), ObjectType::Unknown); }); } diff --git a/guide/src/exception.md b/guide/src/exception.md index 92ac28c5..93498a7a 100644 --- a/guide/src/exception.md +++ b/guide/src/exception.md @@ -79,10 +79,10 @@ use pyo3::Python; use pyo3::types::{PyBool, PyList}; Python::with_gil(|py| { - assert!(PyBool::new(py, true).is_instance_of::().unwrap()); + assert!(PyBool::new(py, true).is_instance_of::()); let list = PyList::new(py, &[1, 2, 3, 4]); - assert!(!list.is_instance_of::().unwrap()); - assert!(list.is_instance_of::().unwrap()); + assert!(!list.is_instance_of::()); + assert!(list.is_instance_of::()); }); ``` diff --git a/newsfragments/2881.changed.md b/newsfragments/2881.changed.md new file mode 100644 index 00000000..9f1e29b4 --- /dev/null +++ b/newsfragments/2881.changed.md @@ -0,0 +1 @@ +`PyAny::is_instance_of::(obj)` is now equivalent to `T::is_type_of(obj)`, and now returns `bool` instead of `PyResult`. diff --git a/src/type_object.rs b/src/type_object.rs index 82e97503..f36abd94 100644 --- a/src/type_object.rs +++ b/src/type_object.rs @@ -47,16 +47,19 @@ pub unsafe trait PyTypeInfo: Sized { fn type_object_raw(py: Python<'_>) -> *mut ffi::PyTypeObject; /// Returns the safe abstraction over the type object. + #[inline] fn type_object(py: Python<'_>) -> &PyType { unsafe { py.from_borrowed_ptr(Self::type_object_raw(py) as _) } } /// Checks if `object` is an instance of this type or a subclass of this type. + #[inline] fn is_type_of(object: &PyAny) -> bool { unsafe { ffi::PyObject_TypeCheck(object.as_ptr(), Self::type_object_raw(object.py())) != 0 } } /// Checks if `object` is an instance of this type. + #[inline] fn is_exact_type_of(object: &PyAny) -> bool { unsafe { ffi::Py_TYPE(object.as_ptr()) == Self::type_object_raw(object.py()) } } diff --git a/src/types/any.rs b/src/types/any.rs index e0abd3b9..e03de936 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -665,6 +665,7 @@ impl PyAny { /// Returns whether the object is considered to be None. /// /// This is equivalent to the Python expression `self is None`. + #[inline] pub fn is_none(&self) -> bool { unsafe { ffi::Py_None() == self.as_ptr() } } @@ -778,7 +779,7 @@ impl PyAny { /// /// Python::with_gil(|py| { /// let dict = PyDict::new(py); - /// assert!(dict.is_instance_of::().unwrap()); + /// assert!(dict.is_instance_of::()); /// let any: &PyAny = dict.as_ref(); /// /// assert!(any.downcast::().is_ok()); @@ -904,6 +905,7 @@ impl PyAny { /// Checks whether this object is an instance of type `ty`. /// /// This is equivalent to the Python expression `isinstance(self, ty)`. + #[inline] pub fn is_instance(&self, ty: &PyAny) -> PyResult { let result = unsafe { ffi::PyObject_IsInstance(self.as_ptr(), ty.as_ptr()) }; err::error_on_minusone(self.py(), result)?; @@ -914,8 +916,9 @@ impl PyAny { /// /// This is equivalent to the Python expression `isinstance(self, T)`, /// if the type `T` is known at compile time. - pub fn is_instance_of(&self) -> PyResult { - self.is_instance(T::type_object(self.py())) + #[inline] + pub fn is_instance_of(&self) -> bool { + T::is_type_of(self) } /// Determines if self contains `value`. @@ -1043,10 +1046,10 @@ class SimpleClass: fn test_any_isinstance_of() { Python::with_gil(|py| { let x = 5.to_object(py).into_ref(py); - assert!(x.is_instance_of::().unwrap()); + assert!(x.is_instance_of::()); let l = vec![x, x].to_object(py).into_ref(py); - assert!(l.is_instance_of::().unwrap()); + assert!(l.is_instance_of::()); }); } diff --git a/src/types/bytes.rs b/src/types/bytes.rs index 1f816d03..ae40c08e 100644 --- a/src/types/bytes.rs +++ b/src/types/bytes.rs @@ -219,10 +219,10 @@ mod tests { .unwrap_err(); let cow = Cow::<[u8]>::Borrowed(b"foobar").to_object(py); - assert!(cow.as_ref(py).is_instance_of::().unwrap()); + assert!(cow.as_ref(py).is_instance_of::()); let cow = Cow::<[u8]>::Owned(b"foobar".to_vec()).to_object(py); - assert!(cow.as_ref(py).is_instance_of::().unwrap()); + assert!(cow.as_ref(py).is_instance_of::()); }); } } diff --git a/src/types/mod.rs b/src/types/mod.rs index 235093df..385d0af6 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -202,6 +202,7 @@ macro_rules! pyobject_native_type_info( } $( + #[inline] fn is_type_of(ptr: &$crate::PyAny) -> bool { use $crate::AsPyPointer; #[allow(unused_unsafe)] diff --git a/src/types/sequence.rs b/src/types/sequence.rs index a6248b37..ccc2d895 100644 --- a/src/types/sequence.rs +++ b/src/types/sequence.rs @@ -297,7 +297,7 @@ where T: FromPyObject<'a>, { fn extract(obj: &'a PyAny) -> PyResult { - if let Ok(true) = obj.is_instance_of::() { + if obj.is_instance_of::() { return Err(PyTypeError::new_err("Can't extract `str` to `Vec`")); } extract_sequence(obj) diff --git a/tests/test_datetime.rs b/tests/test_datetime.rs index 34c2b18d..f863afdc 100644 --- a/tests/test_datetime.rs +++ b/tests/test_datetime.rs @@ -61,9 +61,9 @@ fn test_date_check() { assert_check_exact!(PyDate_Check, PyDate_CheckExact, obj); assert_check_only!(PyDate_Check, PyDate_CheckExact, sub_obj); assert_check_only!(PyDate_Check, PyDate_CheckExact, sub_sub_obj); - assert!(obj.is_instance_of::().unwrap()); - assert!(!obj.is_instance_of::().unwrap()); - assert!(!obj.is_instance_of::().unwrap()); + assert!(obj.is_instance_of::()); + assert!(!obj.is_instance_of::()); + assert!(!obj.is_instance_of::()); }); } @@ -76,9 +76,9 @@ fn test_time_check() { assert_check_exact!(PyTime_Check, PyTime_CheckExact, obj); assert_check_only!(PyTime_Check, PyTime_CheckExact, sub_obj); assert_check_only!(PyTime_Check, PyTime_CheckExact, sub_sub_obj); - assert!(!obj.is_instance_of::().unwrap()); - assert!(obj.is_instance_of::().unwrap()); - assert!(!obj.is_instance_of::().unwrap()); + assert!(!obj.is_instance_of::()); + assert!(obj.is_instance_of::()); + assert!(!obj.is_instance_of::()); }); } @@ -94,9 +94,9 @@ fn test_datetime_check() { assert_check_exact!(PyDateTime_Check, PyDateTime_CheckExact, obj); assert_check_only!(PyDateTime_Check, PyDateTime_CheckExact, sub_obj); assert_check_only!(PyDateTime_Check, PyDateTime_CheckExact, sub_sub_obj); - assert!(obj.is_instance_of::().unwrap()); - assert!(!obj.is_instance_of::().unwrap()); - assert!(obj.is_instance_of::().unwrap()); + assert!(obj.is_instance_of::()); + assert!(!obj.is_instance_of::()); + assert!(obj.is_instance_of::()); }); } diff --git a/tests/test_inheritance.rs b/tests/test_inheritance.rs index 8320eea2..8c46a4de 100644 --- a/tests/test_inheritance.rs +++ b/tests/test_inheritance.rs @@ -113,8 +113,8 @@ fn is_subclass_and_is_instance() { assert!(sub_ty.is_subclass(base_ty).unwrap()); let obj = PyCell::new(py, SubClass::new()).unwrap(); - assert!(obj.is_instance_of::().unwrap()); - assert!(obj.is_instance_of::().unwrap()); + assert!(obj.is_instance_of::()); + assert!(obj.is_instance_of::()); assert!(obj.is_instance(sub_ty).unwrap()); assert!(obj.is_instance(base_ty).unwrap()); });