From 2decb65a3c45b644233cf3b8814323d8a01deb9f Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Thu, 27 Jul 2023 08:55:52 +0100 Subject: [PATCH] optimize `float` -> `f64` conversions on non-abi3 --- benches/bench_frompyobject.rs | 24 +++++++++++++------ newsfragments/3345.changed.md | 1 + pyo3-ffi/src/cpython/floatobject.rs | 27 ++++++++++++++++++++++ pyo3-ffi/src/cpython/mod.rs | 2 ++ pyo3-ffi/src/floatobject.rs | 13 ----------- src/types/floatob.rs | 36 ++++++++++++++++++++--------- 6 files changed, 72 insertions(+), 31 deletions(-) create mode 100644 newsfragments/3345.changed.md create mode 100644 pyo3-ffi/src/cpython/floatobject.rs diff --git a/benches/bench_frompyobject.rs b/benches/bench_frompyobject.rs index 587a89bcdf3..f0f1036ff07 100644 --- a/benches/bench_frompyobject.rs +++ b/benches/bench_frompyobject.rs @@ -2,7 +2,7 @@ use criterion::{black_box, criterion_group, criterion_main, Bencher, Criterion}; use pyo3::{ prelude::*, - types::{PyList, PyString}, + types::{PyFloat, PyList, PyString}, }; #[derive(FromPyObject)] @@ -79,13 +79,23 @@ fn not_a_list_via_extract_enum(b: &mut Bencher<'_>) { }) } +fn f64_from_pyobject(b: &mut Bencher<'_>) { + Python::with_gil(|py| { + let obj = PyFloat::new(py, 1.234); + b.iter(|| { + let _: f64 = obj.extract().unwrap(); + }); + }) +} + fn criterion_benchmark(c: &mut Criterion) { - c.bench_function("enum_from_pyobject", enum_from_pyobject); - c.bench_function("list_via_downcast", list_via_downcast); - c.bench_function("list_via_extract", list_via_extract); - c.bench_function("not_a_list_via_downcast", not_a_list_via_downcast); - 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); + // c.bench_function("enum_from_pyobject", enum_from_pyobject); + // c.bench_function("list_via_downcast", list_via_downcast); + // c.bench_function("list_via_extract", list_via_extract); + // c.bench_function("not_a_list_via_downcast", not_a_list_via_downcast); + // 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); + c.bench_function("f64_from_pyobject", f64_from_pyobject); } criterion_group!(benches, criterion_benchmark); diff --git a/newsfragments/3345.changed.md b/newsfragments/3345.changed.md new file mode 100644 index 00000000000..35143e6abba --- /dev/null +++ b/newsfragments/3345.changed.md @@ -0,0 +1 @@ +Optimize conversion of `float` to `f64` (and `PyFloat::value`) on non-abi3 builds. diff --git a/pyo3-ffi/src/cpython/floatobject.rs b/pyo3-ffi/src/cpython/floatobject.rs new file mode 100644 index 00000000000..e33da0b91b9 --- /dev/null +++ b/pyo3-ffi/src/cpython/floatobject.rs @@ -0,0 +1,27 @@ +use crate::{PyFloat_Check, PyObject}; +use std::os::raw::c_double; + +#[repr(C)] +pub struct PyFloatObject { + pub ob_base: PyObject, + pub ob_fval: c_double, +} + +#[inline] +pub unsafe fn _PyFloat_CAST(op: *mut PyObject) -> *mut PyFloatObject { + debug_assert_eq!(PyFloat_Check(op), 1); + op.cast() +} + +#[inline] +pub unsafe fn PyFloat_AS_DOUBLE(op: *mut PyObject) -> c_double { + (*_PyFloat_CAST(op)).ob_fval +} + +// skipped PyFloat_Pack2 +// skipped PyFloat_Pack4 +// skipped PyFloat_Pack8 + +// skipped PyFloat_Unpack2 +// skipped PyFloat_Unpack4 +// skipped PyFloat_Unpack8 diff --git a/pyo3-ffi/src/cpython/mod.rs b/pyo3-ffi/src/cpython/mod.rs index 7c39a9e4dd6..30c5d3b9a65 100644 --- a/pyo3-ffi/src/cpython/mod.rs +++ b/pyo3-ffi/src/cpython/mod.rs @@ -29,6 +29,7 @@ pub(crate) mod pymem; pub(crate) mod pystate; pub(crate) mod pythonrun; // skipped sysmodule.h +pub(crate) mod floatobject; pub(crate) mod tupleobject; pub(crate) mod unicodeobject; pub(crate) mod weakrefobject; @@ -42,6 +43,7 @@ pub use self::compile::*; pub use self::descrobject::*; #[cfg(not(PyPy))] pub use self::dictobject::*; +pub use self::floatobject::*; pub use self::frameobject::*; pub use self::funcobject::*; pub use self::genobject::*; diff --git a/pyo3-ffi/src/floatobject.rs b/pyo3-ffi/src/floatobject.rs index 176789d3ce0..65fc1d4c316 100644 --- a/pyo3-ffi/src/floatobject.rs +++ b/pyo3-ffi/src/floatobject.rs @@ -6,13 +6,6 @@ use std::ptr::addr_of_mut; // TODO: remove (see https://github.com/PyO3/pyo3/pull/1341#issuecomment-751515985) opaque_struct!(PyFloatObject); -#[cfg(not(Py_LIMITED_API))] -#[repr(C)] -pub struct PyFloatObject { - pub ob_base: PyObject, - pub ob_fval: c_double, -} - #[cfg_attr(windows, link(name = "pythonXY"))] extern "C" { #[cfg_attr(PyPy, link_name = "PyPyFloat_Type")] @@ -44,12 +37,6 @@ extern "C" { pub fn PyFloat_AsDouble(arg1: *mut PyObject) -> c_double; } -#[cfg(not(Py_LIMITED_API))] -#[inline] -pub unsafe fn PyFloat_AS_DOUBLE(op: *mut PyObject) -> c_double { - (*(op as *mut PyFloatObject)).ob_fval -} - // skipped non-limited _PyFloat_Pack2 // skipped non-limited _PyFloat_Pack4 // skipped non-limited _PyFloat_Pack8 diff --git a/src/types/floatob.rs b/src/types/floatob.rs index 1e188079733..4ac425d7962 100644 --- a/src/types/floatob.rs +++ b/src/types/floatob.rs @@ -1,7 +1,8 @@ #[cfg(feature = "experimental-inspect")] use crate::inspect::types::TypeInfo; use crate::{ - ffi, AsPyPointer, FromPyObject, IntoPy, PyAny, PyErr, PyObject, PyResult, Python, ToPyObject, + ffi, AsPyPointer, FromPyObject, IntoPy, PyAny, PyErr, PyNativeType, PyObject, PyResult, Python, + ToPyObject, }; use std::os::raw::c_double; @@ -29,7 +30,16 @@ impl PyFloat { /// Gets the value of this float. pub fn value(&self) -> c_double { - unsafe { ffi::PyFloat_AsDouble(self.as_ptr()) } + #[cfg(not(Py_LIMITED_API))] + unsafe { + // Safety: self is PyFloat object + ffi::PyFloat_AS_DOUBLE(self.as_ptr()) + } + + #[cfg(Py_LIMITED_API)] + unsafe { + ffi::PyFloat_AsDouble(self.as_ptr()) + } } } @@ -54,6 +64,15 @@ impl<'source> FromPyObject<'source> for f64 { // PyFloat_AsDouble returns -1.0 upon failure #![allow(clippy::float_cmp)] fn extract(obj: &'source PyAny) -> PyResult { + // On non-limited API, .value() uses PyFloat_AS_DOUBLE which + // allows us to have an optimized fast path for the case when + // we have exactly a `float` object (it's not worth going through + // `isinstance` machinery for subclasses). + #[cfg(not(Py_LIMITED_API))] + if let Ok(float) = obj.downcast_exact::() { + return Ok(float.value()); + } + let v = unsafe { ffi::PyFloat_AsDouble(obj.as_ptr()) }; if v == -1.0 { @@ -101,11 +120,7 @@ impl<'source> FromPyObject<'source> for f32 { #[cfg(test)] mod tests { - #[cfg(not(Py_LIMITED_API))] - use crate::ffi::PyFloat_AS_DOUBLE; - #[cfg(not(Py_LIMITED_API))] - use crate::AsPyPointer; - use crate::{Python, ToPyObject}; + use crate::{types::PyFloat, Python, ToPyObject}; macro_rules! num_to_py_object_and_back ( ($func_name:ident, $t1:ty, $t2:ty) => ( @@ -127,15 +142,14 @@ mod tests { num_to_py_object_and_back!(to_from_f32, f32, f32); num_to_py_object_and_back!(int_to_float, i32, f64); - #[cfg(not(Py_LIMITED_API))] #[test] - fn test_as_double_macro() { + fn test_float_value() { use assert_approx_eq::assert_approx_eq; Python::with_gil(|py| { let v = 1.23f64; - let obj = v.to_object(py); - assert_approx_eq!(v, unsafe { PyFloat_AS_DOUBLE(obj.as_ptr()) }); + let obj = PyFloat::new(py, 1.23); + assert_approx_eq!(v, obj.value()); }); } }