Merge pull request #2279 from PyO3/extract-error-is-slow

Add benchmark highlighting the costs of failed calls to FromPyObject::extract.

CHANGELOG.md

@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - Default to "m" ABI tag when choosing `libpython` link name for CPython 3.7 on Unix. [#2288](https://github.com/PyO3/pyo3/pull/2288)
+- Improved performance of failing calls to `FromPyObject::extract` which is common when functions accept multiple distinct types. [#2279](https://github.com/PyO3/pyo3/pull/2279)
 
 ### Fixed
 

benches/bench_frompyobject.rs

@@ -1,6 +1,9 @@
-use criterion::{criterion_group, criterion_main, Bencher, Criterion};
+use criterion::{black_box, criterion_group, criterion_main, Bencher, Criterion};
 
-use pyo3::{prelude::*, types::PyString};
+use pyo3::{
+    prelude::*,
+    types::{PyList, PyString},
+};
 
 #[derive(FromPyObject)]
 enum ManyTypes {
@@ -18,8 +21,71 @@ fn enum_from_pyobject(b: &mut Bencher<'_>) {
     })
 }
 
+fn list_via_cast_as(b: &mut Bencher<'_>) {
+    Python::with_gil(|py| {
+        let any: &PyAny = PyList::empty(py).into();
+
+        b.iter(|| {
+            let _list: &PyList = black_box(any).cast_as().unwrap();
+        });
+    })
+}
+
+fn list_via_extract(b: &mut Bencher<'_>) {
+    Python::with_gil(|py| {
+        let any: &PyAny = PyList::empty(py).into();
+
+        b.iter(|| {
+            let _list: &PyList = black_box(any).extract().unwrap();
+        });
+    })
+}
+
+fn not_a_list_via_cast_as(b: &mut Bencher<'_>) {
+    Python::with_gil(|py| {
+        let any: &PyAny = PyString::new(py, "foobar").into();
+
+        b.iter(|| {
+            black_box(any).cast_as::<PyList>().unwrap_err();
+        });
+    })
+}
+
+fn not_a_list_via_extract(b: &mut Bencher<'_>) {
+    Python::with_gil(|py| {
+        let any: &PyAny = PyString::new(py, "foobar").into();
+
+        b.iter(|| {
+            black_box(any).extract::<&PyList>().unwrap_err();
+        });
+    })
+}
+
+#[derive(FromPyObject)]
+enum ListOrNotList<'a> {
+    List(&'a PyList),
+    NotList(&'a PyAny),
+}
+
+fn not_a_list_via_extract_enum(b: &mut Bencher<'_>) {
+    Python::with_gil(|py| {
+        let any: &PyAny = PyString::new(py, "foobar").into();
+
+        b.iter(|| match black_box(any).extract::<ListOrNotList<'_>>() {
+            Ok(ListOrNotList::List(_list)) => panic!(),
+            Ok(ListOrNotList::NotList(_any)) => (),
+            Err(_) => panic!(),
+        });
+    })
+}
+
 fn criterion_benchmark(c: &mut Criterion) {
     c.bench_function("enum_from_pyobject", enum_from_pyobject);
+    c.bench_function("list_via_cast_as", list_via_cast_as);
+    c.bench_function("list_via_extract", list_via_extract);
+    c.bench_function("not_a_list_via_cast_as", not_a_list_via_cast_as);
+    c.bench_function("not_a_list_via_extract", not_a_list_via_extract);
+    c.bench_function("not_a_list_via_extract_enum", not_a_list_via_extract_enum);
 }
 
 criterion_group!(benches, criterion_benchmark);

src/err/mod.rs

@@ -663,10 +663,34 @@ impl<'a> IntoPy<PyObject> for &'a PyErr {
     }
 }
 
+struct PyDowncastErrorArguments {
+    from: Py<PyType>,
+    to: Cow<'static, str>,
+}
+
+impl PyErrArguments for PyDowncastErrorArguments {
+    fn arguments(self, py: Python<'_>) -> PyObject {
+        format!(
+            "'{}' object cannot be converted to '{}'",
+            self.from
+                .as_ref(py)
+                .name()
+                .unwrap_or("<failed to extract type name>"),
+            self.to
+        )
+        .to_object(py)
+    }
+}
+
 /// Convert `PyDowncastError` to Python `TypeError`.
 impl<'a> std::convert::From<PyDowncastError<'a>> for PyErr {
     fn from(err: PyDowncastError<'_>) -> PyErr {
-        exceptions::PyTypeError::new_err(err.to_string())
+        let args = PyDowncastErrorArguments {
+            from: err.from.get_type().into(),
+            to: err.to,
+        };
+
+        exceptions::PyTypeError::new_err(args)
     }
 }