From 206fb59a65f1abf27a5216d57dadfad77b5c46f8 Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Sun, 14 Jan 2024 18:00:16 +0000 Subject: [PATCH] skip slow path on 3.8+ --- newsfragments/3742.changed.md | 2 +- src/conversions/std/num.rs | 42 ++++++++++++++++++++++------------- 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/newsfragments/3742.changed.md b/newsfragments/3742.changed.md index 647d00391f4..bccd5e0cbe9 100644 --- a/newsfragments/3742.changed.md +++ b/newsfragments/3742.changed.md @@ -1 +1 @@ -Improve performance of `extract::` (and other integer types) by checking if types are a `PyLong` and if so avoiding calling `__index__()` before converting the value to an integer, matching the current behavior of `extract::()`. Gives performance improvement of around 30% for successful extraction. +Improve performance of `extract::` (and other integer types) by avoiding call to `__index__()` converting the value to an integer for 3.8+. Gives performance improvement of around 30% for successful extraction. diff --git a/src/conversions/std/num.rs b/src/conversions/std/num.rs index aedf4e0ba54..dff1a72b65c 100644 --- a/src/conversions/std/num.rs +++ b/src/conversions/std/num.rs @@ -1,8 +1,7 @@ #[cfg(feature = "experimental-inspect")] use crate::inspect::types::TypeInfo; use crate::{ - exceptions, ffi, types::PyLong, FromPyObject, IntoPy, PyAny, PyErr, PyObject, PyResult, Python, - ToPyObject, + exceptions, ffi, FromPyObject, IntoPy, PyAny, PyErr, PyObject, PyResult, Python, ToPyObject, }; use std::convert::TryFrom; use std::num::{ @@ -107,21 +106,32 @@ macro_rules! int_fits_c_long { impl<'source> FromPyObject<'source> for $rust_type { fn extract(obj: &'source PyAny) -> PyResult { - // fast path - checking for subclass of `int` just checks a bit in the type object - let val = if let Ok(long) = obj.downcast::() { - err_if_invalid_value(obj.py(), -1, unsafe { ffi::PyLong_AsLong(long.as_ptr()) }) - } else { - unsafe { - let num = ffi::PyNumber_Index(obj.as_ptr()); - if num.is_null() { - Err(PyErr::fetch(obj.py())) - } else { - let val = err_if_invalid_value(obj.py(), -1, ffi::PyLong_AsLong(num)); - ffi::Py_DECREF(num); - val + let val: c_long; + // with python 3.8+ PyLong_AsLong takes care of calling PyNumber_Index itself + // hence no need for the slow `PyNumber_Index` path + // see https://github.com/PyO3/pyo3/pull/3742 for detials. + #[cfg(Py_3_8)] + { + val = err_if_invalid_value(obj.py(), -1, unsafe { ffi::PyLong_AsLong(obj.as_ptr()) })?; + } + #[cfg(not(Py_3_8))] + { + val = if let Ok(long) = obj.downcast::() { + // fast path - checking for subclass of `int` just checks a bit in the type object + err_if_invalid_value(obj.py(), -1, unsafe { ffi::PyLong_AsLong(long.as_ptr()) }) + } else { + unsafe { + let num = ffi::PyNumber_Index(obj.as_ptr()); + if num.is_null() { + Err(PyErr::fetch(obj.py())) + } else { + let val = err_if_invalid_value(obj.py(), -1, ffi::PyLong_AsLong(num)); + ffi::Py_DECREF(num); + val + } } - } - }?; + }?; + } <$rust_type>::try_from(val) .map_err(|e| exceptions::PyOverflowError::new_err(e.to_string())) }