refactor PyAny::is_instance_of for performance

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<ObjectType> {
-    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::<PyBool>()? {
+    } else if obj.is_instance_of::<PyBool>() {
         ObjectType::Bool
-    } else if obj.is_instance_of::<PyByteArray>()? {
+    } else if obj.is_instance_of::<PyByteArray>() {
         ObjectType::ByteArray
-    } else if obj.is_instance_of::<PyBytes>()? {
+    } else if obj.is_instance_of::<PyBytes>() {
         ObjectType::Bytes
-    } else if obj.is_instance_of::<PyDict>()? {
+    } else if obj.is_instance_of::<PyDict>() {
         ObjectType::Dict
-    } else if obj.is_instance_of::<PyFloat>()? {
+    } else if obj.is_instance_of::<PyFloat>() {
         ObjectType::Float
-    } else if obj.is_instance_of::<PyFrozenSet>()? {
+    } else if obj.is_instance_of::<PyFrozenSet>() {
         ObjectType::FrozenSet
-    } else if obj.is_instance_of::<PyInt>()? {
+    } else if obj.is_instance_of::<PyInt>() {
         ObjectType::Int
-    } else if obj.is_instance_of::<PyList>()? {
+    } else if obj.is_instance_of::<PyList>() {
         ObjectType::List
-    } else if obj.is_instance_of::<PySet>()? {
+    } else if obj.is_instance_of::<PySet>() {
         ObjectType::Set
-    } else if obj.is_instance_of::<PyString>()? {
+    } else if obj.is_instance_of::<PyString>() {
         ObjectType::Str
-    } else if obj.is_instance_of::<PyTuple>()? {
+    } else if obj.is_instance_of::<PyTuple>() {
         ObjectType::Tuple
     } else if obj.downcast::<PySequence>().is_ok() {
         ObjectType::Sequence
@@ -58,17 +58,16 @@ fn find_object_type(obj: &PyAny) -> PyResult<ObjectType> {
         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);
     });
 }
 

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::<PyBool>().unwrap());
+    assert!(PyBool::new(py, true).is_instance_of::<PyBool>());
     let list = PyList::new(py, &[1, 2, 3, 4]);
-    assert!(!list.is_instance_of::<PyBool>().unwrap());
-    assert!(list.is_instance_of::<PyList>().unwrap());
+    assert!(!list.is_instance_of::<PyBool>());
+    assert!(list.is_instance_of::<PyList>());
 });
 ```
 

newsfragments/2881.changed.md

@@ -0,0 +1 @@
+`PyAny::is_instance_of::<T>(obj)` is now equivalent to `T::is_type_of(obj)`, and now returns `bool` instead of `PyResult<bool>`.

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()) }
     }

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::<PyAny>().unwrap());
+    ///     assert!(dict.is_instance_of::<PyAny>());
     ///     let any: &PyAny = dict.as_ref();
     ///
     ///     assert!(any.downcast::<PyDict>().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<bool> {
         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<T: PyTypeInfo>(&self) -> PyResult<bool> {
-        self.is_instance(T::type_object(self.py()))
+    #[inline]
+    pub fn is_instance_of<T: PyTypeInfo>(&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::<PyLong>().unwrap());
+            assert!(x.is_instance_of::<PyLong>());
 
             let l = vec![x, x].to_object(py).into_ref(py);
-            assert!(l.is_instance_of::<PyList>().unwrap());
+            assert!(l.is_instance_of::<PyList>());
         });
     }
 

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::<PyBytes>().unwrap());
+            assert!(cow.as_ref(py).is_instance_of::<PyBytes>());
 
             let cow = Cow::<[u8]>::Owned(b"foobar".to_vec()).to_object(py);
-            assert!(cow.as_ref(py).is_instance_of::<PyBytes>().unwrap());
+            assert!(cow.as_ref(py).is_instance_of::<PyBytes>());
         });
     }
 }

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)]

src/types/sequence.rs

@@ -297,7 +297,7 @@ where
     T: FromPyObject<'a>,
 {
     fn extract(obj: &'a PyAny) -> PyResult<Self> {
-        if let Ok(true) = obj.is_instance_of::<PyString>() {
+        if obj.is_instance_of::<PyString>() {
             return Err(PyTypeError::new_err("Can't extract `str` to `Vec`"));
         }
         extract_sequence(obj)

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::<PyDate>().unwrap());
-        assert!(!obj.is_instance_of::<PyTime>().unwrap());
-        assert!(!obj.is_instance_of::<PyDateTime>().unwrap());
+        assert!(obj.is_instance_of::<PyDate>());
+        assert!(!obj.is_instance_of::<PyTime>());
+        assert!(!obj.is_instance_of::<PyDateTime>());
     });
 }
 
@@ -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::<PyDate>().unwrap());
-        assert!(obj.is_instance_of::<PyTime>().unwrap());
-        assert!(!obj.is_instance_of::<PyDateTime>().unwrap());
+        assert!(!obj.is_instance_of::<PyDate>());
+        assert!(obj.is_instance_of::<PyTime>());
+        assert!(!obj.is_instance_of::<PyDateTime>());
     });
 }
 
@@ -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::<PyDate>().unwrap());
-        assert!(!obj.is_instance_of::<PyTime>().unwrap());
-        assert!(obj.is_instance_of::<PyDateTime>().unwrap());
+        assert!(obj.is_instance_of::<PyDate>());
+        assert!(!obj.is_instance_of::<PyTime>());
+        assert!(obj.is_instance_of::<PyDateTime>());
     });
 }
 

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::<SubClass>().unwrap());
-        assert!(obj.is_instance_of::<BaseClass>().unwrap());
+        assert!(obj.is_instance_of::<SubClass>());
+        assert!(obj.is_instance_of::<BaseClass>());
         assert!(obj.is_instance(sub_ty).unwrap());
         assert!(obj.is_instance(base_ty).unwrap());
     });