diff --git a/newsfragments/4456.changed.md b/newsfragments/4456.changed.md new file mode 100644 index 00000000000..094dece12a5 --- /dev/null +++ b/newsfragments/4456.changed.md @@ -0,0 +1 @@ +Improve performance of calls to Python by using the vectorcall calling convention where possible. \ No newline at end of file diff --git a/pyo3-benches/benches/bench_call.rs b/pyo3-benches/benches/bench_call.rs index ca18bbd5e60..ca8c17093af 100644 --- a/pyo3-benches/benches/bench_call.rs +++ b/pyo3-benches/benches/bench_call.rs @@ -2,8 +2,9 @@ use std::hint::black_box; use codspeed_criterion_compat::{criterion_group, criterion_main, Bencher, Criterion}; -use pyo3::prelude::*; use pyo3::ffi::c_str; +use pyo3::prelude::*; +use pyo3::types::IntoPyDict; macro_rules! test_module { ($py:ident, $code:literal) => { @@ -26,6 +27,62 @@ fn bench_call_0(b: &mut Bencher<'_>) { }) } +fn bench_call_1(b: &mut Bencher<'_>) { + Python::with_gil(|py| { + let module = test_module!(py, "def foo(a, b, c): pass"); + + let foo_module = &module.getattr("foo").unwrap(); + let args = ( + <_ as IntoPy>::into_py(1, py).into_bound(py), + <_ as IntoPy>::into_py("s", py).into_bound(py), + <_ as IntoPy>::into_py(1.23, py).into_bound(py), + ); + + b.iter(|| { + for _ in 0..1000 { + black_box(foo_module).call1(args.clone()).unwrap(); + } + }); + }) +} + +fn bench_call(b: &mut Bencher<'_>) { + Python::with_gil(|py| { + let module = test_module!(py, "def foo(a, b, c, d, e): pass"); + + let foo_module = &module.getattr("foo").unwrap(); + let args = ( + <_ as IntoPy>::into_py(1, py).into_bound(py), + <_ as IntoPy>::into_py("s", py).into_bound(py), + <_ as IntoPy>::into_py(1.23, py).into_bound(py), + ); + let kwargs = [("d", 1), ("e", 42)].into_py_dict(py); + + b.iter(|| { + for _ in 0..1000 { + black_box(foo_module) + .call(args.clone(), Some(&kwargs)) + .unwrap(); + } + }); + }) +} + +fn bench_call_one_arg(b: &mut Bencher<'_>) { + Python::with_gil(|py| { + let module = test_module!(py, "def foo(a): pass"); + + let foo_module = &module.getattr("foo").unwrap(); + let arg = <_ as IntoPy>::into_py(1, py).into_bound(py); + + b.iter(|| { + for _ in 0..1000 { + black_box(foo_module).call1((arg.clone(),)).unwrap(); + } + }); + }) +} + fn bench_call_method_0(b: &mut Bencher<'_>) { Python::with_gil(|py| { let module = test_module!( @@ -47,9 +104,96 @@ class Foo: }) } +fn bench_call_method_1(b: &mut Bencher<'_>) { + Python::with_gil(|py| { + let module = test_module!( + py, + " +class Foo: + def foo(self, a, b, c): + pass +" + ); + + let foo_module = &module.getattr("Foo").unwrap().call0().unwrap(); + let args = ( + <_ as IntoPy>::into_py(1, py).into_bound(py), + <_ as IntoPy>::into_py("s", py).into_bound(py), + <_ as IntoPy>::into_py(1.23, py).into_bound(py), + ); + + b.iter(|| { + for _ in 0..1000 { + black_box(foo_module) + .call_method1("foo", args.clone()) + .unwrap(); + } + }); + }) +} + +fn bench_call_method(b: &mut Bencher<'_>) { + Python::with_gil(|py| { + let module = test_module!( + py, + " +class Foo: + def foo(self, a, b, c, d, e): + pass +" + ); + + let foo_module = &module.getattr("Foo").unwrap().call0().unwrap(); + let args = ( + <_ as IntoPy>::into_py(1, py).into_bound(py), + <_ as IntoPy>::into_py("s", py).into_bound(py), + <_ as IntoPy>::into_py(1.23, py).into_bound(py), + ); + let kwargs = [("d", 1), ("e", 42)].into_py_dict(py); + + b.iter(|| { + for _ in 0..1000 { + black_box(foo_module) + .call_method("foo", args.clone(), Some(&kwargs)) + .unwrap(); + } + }); + }) +} + +fn bench_call_method_one_arg(b: &mut Bencher<'_>) { + Python::with_gil(|py| { + let module = test_module!( + py, + " +class Foo: + def foo(self, a): + pass +" + ); + + let foo_module = &module.getattr("Foo").unwrap().call0().unwrap(); + let arg = <_ as IntoPy>::into_py(1, py).into_bound(py); + + b.iter(|| { + for _ in 0..1000 { + black_box(foo_module) + .call_method1("foo", (arg.clone(),)) + .unwrap(); + } + }); + }) +} + fn criterion_benchmark(c: &mut Criterion) { c.bench_function("call_0", bench_call_0); + c.bench_function("call_1", bench_call_1); + c.bench_function("call", bench_call); + c.bench_function("call_one_arg", bench_call_one_arg); c.bench_function("call_method_0", bench_call_method_0); + c.bench_function("call_method_1", bench_call_method_1); + c.bench_function("call_method", bench_call_method); + c.bench_function("call_method_one_arg", bench_call_method_one_arg); } criterion_group!(benches, criterion_benchmark); diff --git a/pyo3-ffi/src/cpython/abstract_.rs b/pyo3-ffi/src/cpython/abstract_.rs index 34525cec16f..83295e58f61 100644 --- a/pyo3-ffi/src/cpython/abstract_.rs +++ b/pyo3-ffi/src/cpython/abstract_.rs @@ -40,7 +40,7 @@ extern "C" { } #[cfg(Py_3_8)] -const PY_VECTORCALL_ARGUMENTS_OFFSET: size_t = +pub const PY_VECTORCALL_ARGUMENTS_OFFSET: size_t = 1 << (8 * std::mem::size_of::() as size_t - 1); #[cfg(Py_3_8)] diff --git a/src/conversion.rs b/src/conversion.rs index d909382bdb9..991f3c56bc1 100644 --- a/src/conversion.rs +++ b/src/conversion.rs @@ -1,10 +1,11 @@ //! Defines conversions between Rust and Python types. use crate::err::PyResult; +use crate::ffi_ptr_ext::FfiPtrExt; #[cfg(feature = "experimental-inspect")] use crate::inspect::types::TypeInfo; use crate::pyclass::boolean_struct::False; use crate::types::any::PyAnyMethods; -use crate::types::PyTuple; +use crate::types::{PyDict, PyString, PyTuple}; use crate::{ ffi, Borrowed, Bound, BoundObject, Py, PyAny, PyClass, PyErr, PyObject, PyRef, PyRefMut, Python, }; @@ -172,6 +173,93 @@ pub trait IntoPy: Sized { fn type_output() -> TypeInfo { TypeInfo::Any } + + // The following methods are helpers to use the vectorcall API where possible. + // They are overridden on tuples to perform a vectorcall. + // Be careful when you're implementing these: they can never refer to `Bound` call methods, + // as those refer to these methods, so this will create an infinite recursion. + #[doc(hidden)] + #[inline] + fn __py_call_vectorcall1<'py>( + self, + py: Python<'py>, + function: Borrowed<'_, 'py, PyAny>, + _: private::Token, + ) -> PyResult> + where + Self: IntoPy>, + { + #[inline] + fn inner<'py>( + py: Python<'py>, + function: Borrowed<'_, 'py, PyAny>, + args: Bound<'py, PyTuple>, + ) -> PyResult> { + unsafe { + ffi::PyObject_Call(function.as_ptr(), args.as_ptr(), std::ptr::null_mut()) + .assume_owned_or_err(py) + } + } + inner( + py, + function, + >>::into_py(self, py).into_bound(py), + ) + } + + #[doc(hidden)] + #[inline] + fn __py_call_vectorcall<'py>( + self, + py: Python<'py>, + function: Borrowed<'_, 'py, PyAny>, + kwargs: Option>, + _: private::Token, + ) -> PyResult> + where + Self: IntoPy>, + { + #[inline] + fn inner<'py>( + py: Python<'py>, + function: Borrowed<'_, 'py, PyAny>, + args: Bound<'py, PyTuple>, + kwargs: Option>, + ) -> PyResult> { + unsafe { + ffi::PyObject_Call( + function.as_ptr(), + args.as_ptr(), + kwargs.map_or_else(std::ptr::null_mut, |kwargs| kwargs.as_ptr()), + ) + .assume_owned_or_err(py) + } + } + inner( + py, + function, + >>::into_py(self, py).into_bound(py), + kwargs, + ) + } + + #[doc(hidden)] + #[inline] + fn __py_call_method_vectorcall1<'py>( + self, + _py: Python<'py>, + object: Borrowed<'_, 'py, PyAny>, + method_name: Borrowed<'_, 'py, PyString>, + _: private::Token, + ) -> PyResult> + where + Self: IntoPy>, + { + // Don't `self.into_py()`! This will lose the optimization of vectorcall. + object + .getattr(method_name) + .and_then(|method| method.call1(self)) + } } /// Defines a conversion from a Rust type to a Python object, which may fail. @@ -502,6 +590,52 @@ impl IntoPy> for () { fn into_py(self, py: Python<'_>) -> Py { PyTuple::empty(py).unbind() } + + #[inline] + fn __py_call_vectorcall1<'py>( + self, + py: Python<'py>, + function: Borrowed<'_, 'py, PyAny>, + _: private::Token, + ) -> PyResult> { + unsafe { ffi::compat::PyObject_CallNoArgs(function.as_ptr()).assume_owned_or_err(py) } + } + + #[inline] + fn __py_call_vectorcall<'py>( + self, + py: Python<'py>, + function: Borrowed<'_, 'py, PyAny>, + kwargs: Option>, + _: private::Token, + ) -> PyResult> { + unsafe { + match kwargs { + Some(kwargs) => ffi::PyObject_Call( + function.as_ptr(), + PyTuple::empty(py).as_ptr(), + kwargs.as_ptr(), + ) + .assume_owned_or_err(py), + None => ffi::compat::PyObject_CallNoArgs(function.as_ptr()).assume_owned_or_err(py), + } + } + } + + #[inline] + #[allow(clippy::used_underscore_binding)] + fn __py_call_method_vectorcall1<'py>( + self, + py: Python<'py>, + object: Borrowed<'_, 'py, PyAny>, + method_name: Borrowed<'_, 'py, PyString>, + _: private::Token, + ) -> PyResult> { + unsafe { + ffi::compat::PyObject_CallMethodNoArgs(object.as_ptr(), method_name.as_ptr()) + .assume_owned_or_err(py) + } + } } impl<'py> IntoPyObject<'py> for () { diff --git a/src/conversions/chrono.rs b/src/conversions/chrono.rs index 1fc3f5ba4b6..42126bcb950 100644 --- a/src/conversions/chrono.rs +++ b/src/conversions/chrono.rs @@ -813,7 +813,7 @@ fn timezone_utc(py: Python<'_>) -> Bound<'_, PyAny> { #[cfg(test)] mod tests { use super::*; - use crate::types::PyTuple; + use crate::{types::PyTuple, BoundObject}; use std::{cmp::Ordering, panic}; #[test] @@ -1333,7 +1333,12 @@ mod tests { .unwrap() .getattr(name) .unwrap() - .call1(args) + .call1( + args.into_pyobject(py) + .map_err(Into::into) + .unwrap() + .into_bound(), + ) .unwrap() } diff --git a/src/instance.rs b/src/instance.rs index ff35fe293a7..5e7b3173759 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -1477,8 +1477,7 @@ impl Py { kwargs: Option<&Bound<'py, PyDict>>, ) -> PyResult where - N: IntoPyObject<'py, Target = PyTuple>, - N::Error: Into, + N: IntoPy>, { self.bind(py).as_any().call(args, kwargs).map(Bound::unbind) } @@ -1486,10 +1485,9 @@ impl Py { /// Calls the object with only positional arguments. /// /// This is equivalent to the Python expression `self(*args)`. - pub fn call1<'py, N>(&self, py: Python<'py>, args: N) -> PyResult + pub fn call1(&self, py: Python<'_>, args: N) -> PyResult where - N: IntoPyObject<'py, Target = PyTuple>, - N::Error: Into, + N: IntoPy>, { self.bind(py).as_any().call1(args).map(Bound::unbind) } @@ -1516,9 +1514,8 @@ impl Py { ) -> PyResult where N: IntoPyObject<'py, Target = PyString>, - A: IntoPyObject<'py, Target = PyTuple>, + A: IntoPy>, N::Error: Into, - A::Error: Into, { self.bind(py) .as_any() @@ -1535,9 +1532,8 @@ impl Py { pub fn call_method1<'py, N, A>(&self, py: Python<'py>, name: N, args: A) -> PyResult where N: IntoPyObject<'py, Target = PyString>, - A: IntoPyObject<'py, Target = PyTuple>, + A: IntoPy>, N::Error: Into, - A::Error: Into, { self.bind(py) .as_any() diff --git a/src/types/any.rs b/src/types/any.rs index 2ad62111a68..801b651ac1a 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -1,5 +1,5 @@ use crate::class::basic::CompareOp; -use crate::conversion::{AsPyPointer, FromPyObjectBound, IntoPyObject}; +use crate::conversion::{private, AsPyPointer, FromPyObjectBound, IntoPy, IntoPyObject}; use crate::err::{DowncastError, DowncastIntoError, PyErr, PyResult}; use crate::exceptions::{PyAttributeError, PyTypeError}; use crate::ffi_ptr_ext::FfiPtrExt; @@ -10,7 +10,7 @@ use crate::type_object::{PyTypeCheck, PyTypeInfo}; #[cfg(not(any(PyPy, GraalPy)))] use crate::types::PySuper; use crate::types::{PyDict, PyIterator, PyList, PyString, PyTuple, PyType}; -use crate::{err, ffi, BoundObject, Python}; +use crate::{err, ffi, BoundObject, Py, Python}; use std::cell::UnsafeCell; use std::cmp::Ordering; use std::os::raw::c_int; @@ -457,8 +457,7 @@ pub trait PyAnyMethods<'py>: crate::sealed::Sealed { /// ``` fn call(&self, args: A, kwargs: Option<&Bound<'_, PyDict>>) -> PyResult> where - A: IntoPyObject<'py, Target = PyTuple>, - A::Error: Into; + A: IntoPy>; /// Calls the object without arguments. /// @@ -513,8 +512,7 @@ pub trait PyAnyMethods<'py>: crate::sealed::Sealed { /// ``` fn call1(&self, args: A) -> PyResult> where - A: IntoPyObject<'py, Target = PyTuple>, - A::Error: Into; + A: IntoPy>; /// Calls a method on the object. /// @@ -561,9 +559,8 @@ pub trait PyAnyMethods<'py>: crate::sealed::Sealed { ) -> PyResult> where N: IntoPyObject<'py, Target = PyString>, - A: IntoPyObject<'py, Target = PyTuple>, - N::Error: Into, - A::Error: Into; + A: IntoPy>, + N::Error: Into; /// Calls a method on the object without arguments. /// @@ -640,9 +637,8 @@ pub trait PyAnyMethods<'py>: crate::sealed::Sealed { fn call_method1(&self, name: N, args: A) -> PyResult> where N: IntoPyObject<'py, Target = PyString>, - A: IntoPyObject<'py, Target = PyTuple>, - N::Error: Into, - A::Error: Into; + A: IntoPy>, + N::Error: Into; /// Returns whether the object is considered to be true. /// @@ -1269,44 +1265,29 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { fn call(&self, args: A, kwargs: Option<&Bound<'_, PyDict>>) -> PyResult> where - A: IntoPyObject<'py, Target = PyTuple>, - A::Error: Into, + A: IntoPy>, { - fn inner<'py>( - any: &Bound<'py, PyAny>, - args: &Bound<'_, PyTuple>, - kwargs: Option<&Bound<'_, PyDict>>, - ) -> PyResult> { - unsafe { - ffi::PyObject_Call( - any.as_ptr(), - args.as_ptr(), - kwargs.map_or(std::ptr::null_mut(), |dict| dict.as_ptr()), - ) - .assume_owned_or_err(any.py()) - } - } - - let py = self.py(); - inner( - self, - &args.into_pyobject(py).map_err(Into::into)?.as_borrowed(), - kwargs, + args.__py_call_vectorcall( + self.py(), + self.as_borrowed(), + kwargs.map(Bound::as_borrowed), + private::Token, ) } + #[inline] fn call0(&self) -> PyResult> { unsafe { ffi::compat::PyObject_CallNoArgs(self.as_ptr()).assume_owned_or_err(self.py()) } } fn call1(&self, args: A) -> PyResult> where - A: IntoPyObject<'py, Target = PyTuple>, - A::Error: Into, + A: IntoPy>, { - self.call(args, None) + args.__py_call_vectorcall1(self.py(), self.as_borrowed(), private::Token) } + #[inline] fn call_method( &self, name: N, @@ -1315,14 +1296,19 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { ) -> PyResult> where N: IntoPyObject<'py, Target = PyString>, - A: IntoPyObject<'py, Target = PyTuple>, + A: IntoPy>, N::Error: Into, - A::Error: Into, { - self.getattr(name) - .and_then(|method| method.call(args, kwargs)) + // Don't `args.into_py()`! This will lose the optimization of vectorcall. + match kwargs { + Some(_) => self + .getattr(name) + .and_then(|method| method.call(args, kwargs)), + None => self.call_method1(name, args), + } } + #[inline] fn call_method0(&self, name: N) -> PyResult> where N: IntoPyObject<'py, Target = PyString>, @@ -1339,11 +1325,17 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { fn call_method1(&self, name: N, args: A) -> PyResult> where N: IntoPyObject<'py, Target = PyString>, - A: IntoPyObject<'py, Target = PyTuple>, + A: IntoPy>, N::Error: Into, - A::Error: Into, { - self.call_method(name, args, None) + args.__py_call_method_vectorcall1( + self.py(), + self.as_borrowed(), + name.into_pyobject(self.py()) + .map_err(Into::into)? + .as_borrowed(), + private::Token, + ) } fn is_truthy(&self) -> PyResult { diff --git a/src/types/tuple.rs b/src/types/tuple.rs index 44a9d15e836..832cf85d2fc 100644 --- a/src/types/tuple.rs +++ b/src/types/tuple.rs @@ -1,13 +1,15 @@ use std::iter::FusedIterator; -use crate::conversion::IntoPyObject; +use crate::conversion::{private, IntoPyObject}; use crate::ffi::{self, Py_ssize_t}; use crate::ffi_ptr_ext::FfiPtrExt; #[cfg(feature = "experimental-inspect")] use crate::inspect::types::TypeInfo; use crate::instance::Borrowed; use crate::internal_tricks::get_ssize_index; -use crate::types::{any::PyAnyMethods, sequence::PySequenceMethods, PyList, PySequence}; +use crate::types::{ + any::PyAnyMethods, sequence::PySequenceMethods, PyDict, PyList, PySequence, PyString, +}; use crate::{ exceptions, Bound, BoundObject, FromPyObject, IntoPy, Py, PyAny, PyErr, PyObject, PyResult, Python, ToPyObject, @@ -522,7 +524,7 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ } #[cfg(feature = "experimental-inspect")] -fn type_output() -> TypeInfo { + fn type_output() -> TypeInfo { TypeInfo::Tuple(Some(vec![$( $T::type_output() ),+])) } } @@ -550,6 +552,109 @@ fn type_output() -> TypeInfo { fn type_output() -> TypeInfo { TypeInfo::Tuple(Some(vec![$( $T::type_output() ),+])) } + + #[inline] + fn __py_call_vectorcall1<'py>( + self, + py: Python<'py>, + function: Borrowed<'_, 'py, PyAny>, + _: private::Token, + ) -> PyResult> { + cfg_if::cfg_if! { + if #[cfg(all(Py_3_9, not(any(PyPy, GraalPy, Py_LIMITED_API))))] { + // We need this to drop the arguments correctly. + let args_bound = [$(self.$n.into_py(py).into_bound(py),)*]; + if $length == 1 { + unsafe { + ffi::PyObject_CallOneArg(function.as_ptr(), args_bound[0].as_ptr()).assume_owned_or_err(py) + } + } else { + // Prepend one null argument for `PY_VECTORCALL_ARGUMENTS_OFFSET`. + let mut args = [std::ptr::null_mut(), $(args_bound[$n].as_ptr()),*]; + unsafe { + ffi::PyObject_Vectorcall( + function.as_ptr(), + args.as_mut_ptr().add(1), + $length + ffi::PY_VECTORCALL_ARGUMENTS_OFFSET, + std::ptr::null_mut(), + ) + .assume_owned_or_err(py) + } + } + } else { + function.call1(>>::into_py(self, py).into_bound(py)) + } + } + } + + #[inline] + fn __py_call_vectorcall<'py>( + self, + py: Python<'py>, + function: Borrowed<'_, 'py, PyAny>, + kwargs: Option>, + _: private::Token, + ) -> PyResult> { + cfg_if::cfg_if! { + if #[cfg(all(Py_3_9, not(any(PyPy, GraalPy, Py_LIMITED_API))))] { + // We need this to drop the arguments correctly. + let args_bound = [$(self.$n.into_py(py).into_bound(py),)*]; + // Prepend one null argument for `PY_VECTORCALL_ARGUMENTS_OFFSET`. + let mut args = [std::ptr::null_mut(), $(args_bound[$n].as_ptr()),*]; + unsafe { + ffi::PyObject_VectorcallDict( + function.as_ptr(), + args.as_mut_ptr().add(1), + $length + ffi::PY_VECTORCALL_ARGUMENTS_OFFSET, + kwargs.map_or_else(std::ptr::null_mut, |kwargs| kwargs.as_ptr()), + ) + .assume_owned_or_err(py) + } + } else { + function.call(>>::into_py(self, py).into_bound(py), kwargs.as_deref()) + } + } + } + + #[inline] + fn __py_call_method_vectorcall1<'py>( + self, + py: Python<'py>, + object: Borrowed<'_, 'py, PyAny>, + method_name: Borrowed<'_, 'py, PyString>, + _: private::Token, + ) -> PyResult> { + cfg_if::cfg_if! { + if #[cfg(all(Py_3_9, not(any(PyPy, GraalPy, Py_LIMITED_API))))] { + // We need this to drop the arguments correctly. + let args_bound = [$(self.$n.into_py(py).into_bound(py),)*]; + if $length == 1 { + unsafe { + ffi::PyObject_CallMethodOneArg( + object.as_ptr(), + method_name.as_ptr(), + args_bound[0].as_ptr(), + ) + .assume_owned_or_err(py) + } + } else { + let mut args = [object.as_ptr(), $(args_bound[$n].as_ptr()),*]; + unsafe { + ffi::PyObject_VectorcallMethod( + method_name.as_ptr(), + args.as_mut_ptr(), + // +1 for the receiver. + 1 + $length + ffi::PY_VECTORCALL_ARGUMENTS_OFFSET, + std::ptr::null_mut(), + ) + .assume_owned_or_err(py) + } + } + } else { + object.call_method1(method_name, >>::into_py(self, py).into_bound(py)) + } + } + } } impl<'py, $($T: FromPyObject<'py>),+> FromPyObject<'py> for ($($T,)+) { @@ -568,7 +673,7 @@ fn type_output() -> TypeInfo { } #[cfg(feature = "experimental-inspect")] -fn type_input() -> TypeInfo { + fn type_input() -> TypeInfo { TypeInfo::Tuple(Some(vec![$( $T::type_input() ),+])) } }