From 40709db801abcc991ea2ddf52c6300e463e9baa6 Mon Sep 17 00:00:00 2001
From: David Hewitt <1939362+davidhewitt@users.noreply.github.com>
Date: Wed, 15 Feb 2023 08:00:05 +0000
Subject: [PATCH] optimize mapping conversion for dict

---
 newsfragments/2954.changed.md |  1 +
 src/types/mapping.rs          | 37 ++++++++++++++++-------------------
 2 files changed, 18 insertions(+), 20 deletions(-)
 create mode 100644 newsfragments/2954.changed.md

diff --git a/newsfragments/2954.changed.md b/newsfragments/2954.changed.md
new file mode 100644
index 00000000..a7760a62
--- /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 ab478ba7..59300ac7 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<PyResult<Py<PyType>>> = 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<Py<PyType>> = GILOnceCell::new();
+
+fn get_mapping_abc(py: Python<'_>) -> PyResult<&PyType> {
     MAPPING_ABC
-        .get_or_init(py, || {
-            Ok(py
-                .import("collections.abc")?
-                .getattr("Mapping")?
-                .downcast::<PyType>()?
-                .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<V: Into<&'v PyAny>>(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"))