From e48399c456ae712329edcf21814f465130bf1dc2 Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Thu, 19 Nov 2020 14:34:44 +0000 Subject: [PATCH] performance: use vectorcall for call0 and call_method0 --- CHANGELOG.md | 5 ++-- Cargo.toml | 1 + src/instance.rs | 25 +++++++++++++++++-- src/types/any.rs | 63 ++++++++++++++++++++++++++++++++++++++++++++---- 4 files changed, 85 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 400a8892041..17453ec7d7d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Add FFI definitions for PEP 587 "Python Initialization Configuration". [#1247](https://github.com/PyO3/pyo3/pull/1247) - Add `PyEval_SetProfile` and `PyEval_SetTrace` to FFI. [#1255](https://github.com/PyO3/pyo3/pull/1255) - Add context.h functions (`PyContext_New`, etc) to FFI. [#1259](https://github.com/PyO3/pyo3/pull/1259) -- Add FFI definitions for `PyBuffer_SizeFromFormat`, `PyObject_LengthHint`, `PyObject_CallNoArgs`, `PyObject_CallOneArg`, `PyObject_CallMethodNoArgs`, `PyObject_CallMethodOneArg`, `PyObject_VectorcallDict`, and `PyObject_VectorcallMethod`. [#1285](https://github.com/PyO3/pyo3/pull/1285) +- Add FFI definitions for `PyBuffer_SizeFromFormat`, `PyObject_LengthHint`, `PyObject_CallNoArgs`, `PyObject_CallOneArg`, `PyObject_CallMethodNoArgs`, `PyObject_CallMethodOneArg`, `PyObject_VectorcallDict`, and `PyObject_VectorcallMethod`. [#1287](https://github.com/PyO3/pyo3/pull/1285) ### Changed - Change return type `PyType::name()` from `Cow` to `PyResult<&str>`. [#1152](https://github.com/PyO3/pyo3/pull/1152) @@ -28,6 +28,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Change `Debug` and `Display` impls for `PyException` to be consistent with `PyAny`. [#1275](https://github.com/PyO3/pyo3/pull/1275) - Change `Debug` impl of `PyErr` to output more helpful information (acquiring the GIL if necessary). [#1275](https://github.com/PyO3/pyo3/pull/1275) - Rename `PyTypeInfo::is_instance` and `PyTypeInfo::is_exact_instance` to `PyTypeInfo::is_type_of` and `PyTypeInfo::is_exact_type_of`. [#1278](https://github.com/PyO3/pyo3/pull/1278) +- Improve performance of `PyAny::call0`, `Py::call0` and `PyAny::call_method0` and `Py::call_method0` on Python 3.9 and up. [#1287](https://github.com/PyO3/pyo3/pull/1285) ### Removed - Remove deprecated ffi definitions `PyUnicode_AsUnicodeCopy`, `PyUnicode_GetMax`, `_Py_CheckRecursionLimit`, `PyObject_AsCharBuffer`, `PyObject_AsReadBuffer`, `PyObject_CheckReadBuffer` and `PyObject_AsWriteBuffer`, which will be removed in Python 3.10. [#1217](https://github.com/PyO3/pyo3/pull/1217) @@ -36,7 +37,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed - Fix missing field in `PyCodeObject` struct (`co_posonlyargcount`) - caused invalid access to other fields in Python >3.7. [#1260](https://github.com/PyO3/pyo3/pull/1260) - Fix building for `x86_64-unknown-linux-musl` target from `x86_65-unknown-linux-gnu` host. [#1267](https://github.com/PyO3/pyo3/pull/1267) -- Fix FFI definitions for `PyObject_Vectorcall` and `PyVectorcall_Call`. [#1285](https://github.com/PyO3/pyo3/pull/1285) +- Fix FFI definitions for `PyObject_Vectorcall` and `PyVectorcall_Call`. [#1287](https://github.com/PyO3/pyo3/pull/1285) ## [0.12.3] - 2020-10-12 ### Fixed diff --git a/Cargo.toml b/Cargo.toml index 29a2674140d..9850271f9df 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,6 +15,7 @@ build = "build.rs" edition = "2018" [dependencies] +cfg-if = { version = "1.0" } ctor = { version = "0.1", optional = true } indoc = { version = "1.0.3", optional = true } inventory = { version = "0.1.4", optional = true } diff --git a/src/instance.rs b/src/instance.rs index 9cfc40026b4..78379ceefc1 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -272,7 +272,18 @@ impl Py { /// /// This is equivalent to the Python expression `self()`. pub fn call0(&self, py: Python) -> PyResult { - self.call(py, (), None) + cfg_if::cfg_if! { + // TODO: Use PyObject_CallNoArgs instead after https://bugs.python.org/issue42415, + // at which point can enable optimization for limited API. + if #[cfg(all(Py_3_9, not(Py_LIMITED_API)))] { + // Optimized path on python 3.9+ + unsafe { + PyObject::from_owned_ptr_or_err(py, ffi::_PyObject_CallNoArg(self.as_ptr())) + } + } else { + self.call(py, (), None) + } + } } /// Calls a method on the object. @@ -316,7 +327,17 @@ impl Py { /// /// This is equivalent to the Python expression `self.name()`. pub fn call_method0(&self, py: Python, name: &str) -> PyResult { - self.call_method(py, name, (), None) + cfg_if::cfg_if! { + if #[cfg(all(Py_3_9, not(Py_LIMITED_API)))] { + // Optimized path on python 3.9+ + unsafe { + let name = name.into_py(py); + PyObject::from_owned_ptr_or_err(py, ffi::PyObject_CallMethodNoArgs(self.as_ptr(), name.as_ptr())) + } + } else { + self.call_method(py, name, (), None) + } + } } /// Create a `Py` instance by taking ownership of the given FFI pointer. diff --git a/src/types/any.rs b/src/types/any.rs index faf0b119933..05d655de32f 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -225,7 +225,18 @@ impl PyAny { /// /// This is equivalent to the Python expression `self()`. pub fn call0(&self) -> PyResult<&PyAny> { - self.call((), None) + cfg_if::cfg_if! { + // TODO: Use PyObject_CallNoArgs instead after https://bugs.python.org/issue42415, + // at which point can enable optimization for limited API. + if #[cfg(all(Py_3_9, not(Py_LIMITED_API)))] { + // Optimized path on python 3.9+ + unsafe { + self.py().from_owned_ptr_or_err(ffi::_PyObject_CallNoArg(self.as_ptr())) + } + } else { + self.call((), None) + } + } } /// Calls the object with only positional arguments. @@ -282,7 +293,17 @@ impl PyAny { /// /// This is equivalent to the Python expression `self.name()`. pub fn call_method0(&self, name: &str) -> PyResult<&PyAny> { - self.call_method(name, (), None) + cfg_if::cfg_if! { + if #[cfg(all(Py_3_9, not(Py_LIMITED_API)))] { + // Optimized path on python 3.9+ + unsafe { + let name = name.into_py(self.py()); + self.py().from_owned_ptr_or_err(ffi::PyObject_CallMethodNoArgs(self.as_ptr(), name.as_ptr())) + } + } else { + self.call_method(name, (), None) + } + } } /// Calls a method on the object with only positional arguments. @@ -456,9 +477,17 @@ impl PyAny { #[cfg(test)] mod test { - use crate::types::{IntoPyDict, PyList}; - use crate::Python; - use crate::ToPyObject; + use crate::{ + types::{IntoPyDict, PyList, PyModule}, + Python, ToPyObject, + }; + + macro_rules! test_module { + ($py:ident, $code:literal) => { + PyModule::from_code($py, indoc::indoc!($code), file!(), "test_module") + .expect("module creation failed") + }; + } #[test] fn test_call_for_non_existing_method() { @@ -481,6 +510,30 @@ mod test { assert_eq!(list.extract::>(py).unwrap(), vec![7, 6, 5, 4, 3]); } + #[test] + fn test_call_method0() { + Python::with_gil(|py| { + let module = test_module!( + py, + r#" + class SimpleClass: + def foo(self): + return 42 + "# + ); + + let simple_class = module.getattr("SimpleClass").unwrap().call0().unwrap(); + assert_eq!( + simple_class + .call_method0("foo") + .unwrap() + .extract::() + .unwrap(), + 42 + ); + }) + } + #[test] fn test_type() { let gil = Python::acquire_gil();