diff --git a/benches/bench_dict.rs b/benches/bench_dict.rs index f491b653deb..2b92159d10a 100644 --- a/benches/bench_dict.rs +++ b/benches/bench_dict.rs @@ -1,7 +1,7 @@ use criterion::{criterion_group, criterion_main, Bencher, Criterion}; -use pyo3::prelude::*; use pyo3::types::IntoPyDict; +use pyo3::{prelude::*, types::PyMapping}; use std::collections::{BTreeMap, HashMap}; fn iter_dict(b: &mut Bencher<'_>) { @@ -63,6 +63,19 @@ fn extract_hashbrown_map(b: &mut Bencher<'_>) { }); } +fn mapping_from_dict(b: &mut Bencher<'_>) { + Python::with_gil(|py| { + const LEN: usize = 100_000; + let dict = (0..LEN as u64) + .map(|i| (i, i * 2)) + .into_py_dict(py) + .to_object(py); + b.iter(|| { + let _: &PyMapping = dict.extract(py).unwrap(); + }); + }); +} + fn criterion_benchmark(c: &mut Criterion) { c.bench_function("iter_dict", iter_dict); c.bench_function("dict_new", dict_new); @@ -72,6 +85,8 @@ fn criterion_benchmark(c: &mut Criterion) { #[cfg(feature = "hashbrown")] c.bench_function("extract_hashbrown_map", extract_hashbrown_map); + + c.bench_function("mapping_from_dict", mapping_from_dict); } criterion_group!(benches, criterion_benchmark); diff --git a/newsfragments/2954.changed.md b/newsfragments/2954.changed.md new file mode 100644 index 00000000000..a7760a622e8 --- /dev/null +++ b/newsfragments/2954.changed.md @@ -0,0 +1 @@ +Optimize `PyMapping` conversion for `dict` inputs. diff --git a/src/types/mapping.rs b/src/types/mapping.rs index ab478ba72af..59300ac702b 100644 --- a/src/types/mapping.rs +++ b/src/types/mapping.rs @@ -3,12 +3,8 @@ use crate::err::{PyDowncastError, PyErr, PyResult}; use crate::once_cell::GILOnceCell; use crate::type_object::PyTypeInfo; -use crate::types::{PyAny, PySequence, PyType}; -use crate::{ - ffi, AsPyPointer, IntoPy, IntoPyPointer, Py, PyNativeType, PyTryFrom, Python, ToPyObject, -}; - -static MAPPING_ABC: GILOnceCell>> = GILOnceCell::new(); +use crate::types::{PyAny, PyDict, PySequence, PyType}; +use crate::{ffi, AsPyPointer, IntoPyPointer, Py, PyNativeType, PyTryFrom, Python, ToPyObject}; /// Represents a reference to a Python object supporting the mapping protocol. #[repr(transparent)] @@ -119,17 +115,14 @@ impl PyMapping { } } -fn get_mapping_abc(py: Python<'_>) -> Result<&PyType, PyErr> { +static MAPPING_ABC: GILOnceCell> = GILOnceCell::new(); + +fn get_mapping_abc(py: Python<'_>) -> PyResult<&PyType> { MAPPING_ABC - .get_or_init(py, || { - Ok(py - .import("collections.abc")? - .getattr("Mapping")? - .downcast::()? - .into_py(py)) + .get_or_try_init(py, || { + py.import("collections.abc")?.getattr("Mapping")?.extract() }) - .as_ref() - .map_or_else(|e| Err(e.clone_ref(py)), |t| Ok(t.as_ref(py))) + .map(|ty| ty.as_ref(py)) } impl<'v> PyTryFrom<'v> for PyMapping { @@ -139,11 +132,15 @@ impl<'v> PyTryFrom<'v> for PyMapping { fn try_from>(value: V) -> Result<&'v PyMapping, PyDowncastError<'v>> { let value = value.into(); - // TODO: surface specific errors in this chain to the user - if let Ok(abc) = get_mapping_abc(value.py()) { - if value.is_instance(abc).unwrap_or(false) { - unsafe { return Ok(value.downcast_unchecked()) } - } + // Using `is_instance` for `collections.abc.Mapping` is slow, so provide + // optimized case dict as a well-known mapping + if PyDict::is_type_of(value) + || get_mapping_abc(value.py()) + .and_then(|abc| value.is_instance(abc)) + // TODO: surface errors in this chain to the user + .unwrap_or(false) + { + unsafe { return Ok(value.downcast_unchecked()) } } Err(PyDowncastError::new(value, "Mapping"))