Skip to content

Commit

Permalink
refactor PyAny::is_instance_of for performance
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhewitt committed May 18, 2023
1 parent 941ee18 commit 248230b
Show file tree
Hide file tree
Showing 10 changed files with 47 additions and 40 deletions.
35 changes: 17 additions & 18 deletions benches/bench_any.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -27,48 +27,47 @@ 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
} else if obj.downcast::<PyMapping>().is_ok() {
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);
});
}

Expand Down
6 changes: 3 additions & 3 deletions guide/src/exception.md
Original file line number Diff line number Diff line change
Expand Up @@ -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>());
});
```

Expand Down
1 change: 1 addition & 0 deletions newsfragments/2881.changed.md
Original file line number Diff line number Diff line change
@@ -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>`.
3 changes: 3 additions & 0 deletions src/type_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()) }
}
Expand Down
13 changes: 8 additions & 5 deletions src/types/any.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() }
}
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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)?;
Expand All @@ -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`.
Expand Down Expand Up @@ -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>());
});
}

Expand Down
4 changes: 2 additions & 2 deletions src/types/bytes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>());
});
}
}
1 change: 1 addition & 0 deletions src/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
2 changes: 1 addition & 1 deletion src/types/sequence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
18 changes: 9 additions & 9 deletions tests/test_datetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>());
});
}

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

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

Expand Down
4 changes: 2 additions & 2 deletions tests/test_inheritance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
});
Expand Down

0 comments on commit 248230b

Please sign in to comment.