Merge #2881

2881: Rework `PyAny::is_instance_of` for performance r=adamreichold a=davidhewitt

I was talking to `@samuelcolvin` about the fastest way to identify object types (relevant e.g. for `pythonize` and also `pydantic-core`) and noticed that `PyAny::is_instance_of` is quite unoptimised because it expands to an ffi call to `PyObject_IsInstance`.

This PR proposes `PyAny::is_instance_of::<T>(obj)` is changed to be equivalent to `T::is_type_of(obj)`, plus add a sprinkling of inlining. We often have implementations such as `PyDict_Check` which can pretty much be optimised away to just checking a bit on the type object.

The accompanying benchmark to run through a bunch of object types is approx 40% faster after making this change.

For completeness, I've also added `PyAny::is_exact_instance` and `PyAny::is_exact_instance_of`, to pair with `T::is_exact_type_of`. (This could be split into a separate PR if preferred.)



Co-authored-by: David Hewitt <[email protected]>

Author: bors[bot]

Cargo.toml

@@ -114,6 +114,10 @@ full = [
     "rust_decimal",
 ]
 
+[[bench]]
+name = "bench_any"
+harness = false
+
 [[bench]]
 name = "bench_call"
 harness = false

benches/bench_any.rs

@@ -0,0 +1,79 @@
+use criterion::{criterion_group, criterion_main, Bencher, Criterion};
+
+use pyo3::{
+    types::{
+        PyBool, PyByteArray, PyBytes, PyDict, PyFloat, PyFrozenSet, PyInt, PyList, PyMapping,
+        PySequence, PySet, PyString, PyTuple,
+    },
+    PyAny, Python,
+};
+
+#[derive(PartialEq, Eq, Debug)]
+enum ObjectType {
+    None,
+    Bool,
+    ByteArray,
+    Bytes,
+    Dict,
+    Float,
+    FrozenSet,
+    Int,
+    List,
+    Set,
+    Str,
+    Tuple,
+    Sequence,
+    Mapping,
+    Unknown,
+}
+
+fn find_object_type(obj: &PyAny) -> ObjectType {
+    if obj.is_none() {
+        ObjectType::None
+    } else if obj.is_instance_of::<PyBool>() {
+        ObjectType::Bool
+    } else if obj.is_instance_of::<PyByteArray>() {
+        ObjectType::ByteArray
+    } else if obj.is_instance_of::<PyBytes>() {
+        ObjectType::Bytes
+    } else if obj.is_instance_of::<PyDict>() {
+        ObjectType::Dict
+    } else if obj.is_instance_of::<PyFloat>() {
+        ObjectType::Float
+    } else if obj.is_instance_of::<PyFrozenSet>() {
+        ObjectType::FrozenSet
+    } else if obj.is_instance_of::<PyInt>() {
+        ObjectType::Int
+    } else if obj.is_instance_of::<PyList>() {
+        ObjectType::List
+    } else if obj.is_instance_of::<PySet>() {
+        ObjectType::Set
+    } else if obj.is_instance_of::<PyString>() {
+        ObjectType::Str
+    } else if obj.is_instance_of::<PyTuple>() {
+        ObjectType::Tuple
+    } else if obj.downcast::<PySequence>().is_ok() {
+        ObjectType::Sequence
+    } else if obj.downcast::<PyMapping>().is_ok() {
+        ObjectType::Mapping
+    } else {
+        ObjectType::Unknown
+    }
+}
+
+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));
+
+        assert_eq!(find_object_type(obj), ObjectType::Unknown);
+    });
+}
+
+fn criterion_benchmark(c: &mut Criterion) {
+    c.bench_function("identify_object_type", bench_identify_object_type);
+}
+
+criterion_group!(benches, criterion_benchmark);
+criterion_main!(benches);

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