Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pyfunction: refactor argument extraction #1440

Merged
merged 4 commits into from
Mar 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- `PYO3_CROSS_LIB_DIR` enviroment variable no long required when compiling for x86-64 Python from macOS arm64 and reverse. [#1428](/~https://github.com/PyO3/pyo3/pull/1428)
- Fix FFI definition `_PyEval_RequestCodeExtraIndex` which took an argument of the wrong type. [#1429](/~https://github.com/PyO3/pyo3/pull/1429)
- Fix FFI definition `PyIndex_Check` missing with the `abi3` feature. [#1436](/~https://github.com/PyO3/pyo3/pull/1436)
- Fix incorrect `TypeError` raised when keyword-only argument passed along with a positional argument in `*args`. [#1440](/~https://github.com/PyO3/pyo3/pull/1440)
- Fix inability to use a named lifetime for `&PyTuple` of `*args` in `#[pyfunction]`. [#1440](/~https://github.com/PyO3/pyo3/pull/1440)

## [0.13.2] - 2021-02-12
### Packaging
Expand Down
16 changes: 16 additions & 0 deletions examples/pyo3_benchmarks/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
[package]
authors = ["PyO3 Authors"]
name = "pyo3-benchmarks"
version = "0.1.0"
description = "Python-based benchmarks for various PyO3 functionality"
edition = "2018"

[dependencies]

[dependencies.pyo3]
path = "../../"
features = ["extension-module"]

[lib]
name = "_pyo3_benchmarks"
crate-type = ["cdylib"]
2 changes: 2 additions & 0 deletions examples/pyo3_benchmarks/MANIFEST.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
include pyproject.toml Cargo.toml
recursive-include src *
17 changes: 17 additions & 0 deletions examples/pyo3_benchmarks/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# rustapi_module

A simple extension module built using PyO3.

## Build

```shell
python setup.py install
```

## Testing

To test install tox globally and run

```shell
tox -e py
```
1 change: 1 addition & 0 deletions examples/pyo3_benchmarks/pyo3_benchmarks/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
from ._pyo3_benchmarks import *
6 changes: 6 additions & 0 deletions examples/pyo3_benchmarks/requirements-dev.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pip>=19.1
hypothesis>=3.55
pytest>=3.5.0
setuptools-rust>=0.10.2
psutil>=5.6
pytest-benchmark~=3.2
44 changes: 44 additions & 0 deletions examples/pyo3_benchmarks/setup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import sys
import platform

from setuptools import setup
from setuptools_rust import RustExtension


def get_py_version_cfgs():
# For now each Cfg Py_3_X flag is interpreted as "at least 3.X"
version = sys.version_info[0:2]
py3_min = 6
out_cfg = []
for minor in range(py3_min, version[1] + 1):
out_cfg.append("--cfg=Py_3_%d" % minor)

if platform.python_implementation() == "PyPy":
out_cfg.append("--cfg=PyPy")

return out_cfg


setup(
name="pyo3-benchmarks",
version="0.1.0",
classifiers=[
"License :: OSI Approved :: MIT License",
"Development Status :: 3 - Alpha",
"Intended Audience :: Developers",
"Programming Language :: Python",
"Programming Language :: Rust",
"Operating System :: POSIX",
"Operating System :: MacOS :: MacOS X",
],
packages=["pyo3_benchmarks"],
rust_extensions=[
RustExtension(
"pyo3_benchmarks._pyo3_benchmarks",
rustc_flags=get_py_version_cfgs(),
debug=False,
),
],
include_package_data=True,
zip_safe=False,
)
33 changes: 33 additions & 0 deletions examples/pyo3_benchmarks/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
use pyo3::prelude::*;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how this benchmark is useful.
It may be difficult to observe some speedup when trying to extract only a few args.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually although the variance is quite high, there's still a reasonably clear signal as to how good a change is using these benchmarks. I wouldn't take these numbers super seriously for any kind of scientific conclusion. However I can see (for example) that there's still some overhead calling pyo3 functions vs CPython functions, and also that returning Vec from extract_keyword_arguments leads to noticeable slowdown.

Some example numbers I see at the moment:

-------------------------------------------------------------------------------------------- benchmark: 6 tests -------------------------------------------------------------------------------------------
Name (time in ns)                Min                    Max                Mean              StdDev              Median                IQR             Outliers  OPS (Mops/s)            Rounds  Iterations
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_no_args_py              60.0000 (1.0)       4,658.0000 (2.86)      67.6030 (1.0)       34.3033 (1.08)      66.0000 (1.0)       2.0000 (inf)      1857;3556       14.7923 (1.0)      149254         100
test_no_args                 70.0000 (1.17)      1,627.0000 (1.0)       78.7283 (1.16)      31.8182 (1.0)       74.0000 (1.12)      2.0000 (inf)      3261;8374       12.7019 (0.86)     128206         100
test_args_and_kwargs_py     219.0476 (3.65)      5,566.6667 (3.42)     245.2637 (3.63)      57.6773 (1.81)     242.8571 (3.68)      4.7619 (inf)     1156;10254        4.0772 (0.28)     196079          21
test_mixed_args_py          288.2353 (4.80)      6,329.4118 (3.89)     320.9264 (4.75)      83.6458 (2.63)     317.6471 (4.81)      5.8824 (inf)     1406;18940        3.1160 (0.21)     192308          17
test_args_and_kwargs        299.9999 (5.00)     86,200.0001 (52.98)    412.2963 (6.10)     466.4672 (14.66)    400.0000 (6.06)      0.0000 (1.0)      359;47106        2.4254 (0.16)     163935           1
test_mixed_args             340.0000 (5.67)     21,400.0000 (13.15)    385.6509 (5.70)     207.9585 (6.54)     380.0000 (5.76)     15.0000 (inf)      1187;1616        2.5930 (0.18)     128206          20
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

At the moment it's clear we're a bit slower than CPython for function calls, but I hope vectorcall support and #1308 will improve things.

Later I would like to add more numbers to these benchmarks, for example #661.

use pyo3::types::{PyDict, PyTuple};
use pyo3::wrap_pyfunction;

#[pyfunction(args = "*", kwargs = "**")]
fn args_and_kwargs<'a>(
args: &'a PyTuple,
kwargs: Option<&'a PyDict>,
) -> (&'a PyTuple, Option<&'a PyDict>) {
(args, kwargs)
}

#[pyfunction(a, b = 2, args = "*", c = 4, kwargs = "**")]
fn mixed_args<'a>(
a: i32,
b: i32,
args: &'a PyTuple,
c: i32,
kwargs: Option<&'a PyDict>,
) -> (i32, i32, &'a PyTuple, i32, Option<&'a PyDict>) {
(a, b, args, c, kwargs)
}

#[pyfunction]
fn no_args() {}

#[pymodule]
fn _pyo3_benchmarks(_py: Python<'_>, m: &PyModule) -> PyResult<()> {
m.add_function(wrap_pyfunction!(args_and_kwargs, m)?)?;
m.add_function(wrap_pyfunction!(mixed_args, m)?)?;
m.add_function(wrap_pyfunction!(no_args, m)?)?;
Ok(())
}
46 changes: 46 additions & 0 deletions examples/pyo3_benchmarks/tests/test_benchmarks.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import pyo3_benchmarks


def test_args_and_kwargs(benchmark):
benchmark(pyo3_benchmarks.args_and_kwargs, 1, 2, 3, a=4, foo=10)


def args_and_kwargs_py(*args, **kwargs):
return (args, kwargs)


def test_args_and_kwargs_py(benchmark):
rust = pyo3_benchmarks.args_and_kwargs(1, 2, 3, bar=4, foo=10)
py = args_and_kwargs_py(1, 2, 3, bar=4, foo=10)
assert rust == py
benchmark(args_and_kwargs_py, 1, 2, 3, bar=4, foo=10)


def test_mixed_args(benchmark):
benchmark(pyo3_benchmarks.mixed_args, 1, 2, 3, bar=4, foo=10)


def mixed_args_py(a, b=2, *args, c=4, **kwargs):
return (a, b, args, c, kwargs)


def test_mixed_args_py(benchmark):
rust = pyo3_benchmarks.mixed_args(1, 2, 3, bar=4, foo=10)
py = mixed_args_py(1, 2, 3, bar=4, foo=10)
assert rust == py
benchmark(mixed_args_py, 1, 2, 3, bar=4, foo=10)


def test_no_args(benchmark):
benchmark(pyo3_benchmarks.no_args)


def no_args_py():
return None


def test_no_args_py(benchmark):
rust = pyo3_benchmarks.no_args()
py = no_args_py()
assert rust == py
benchmark(no_args_py)
10 changes: 10 additions & 0 deletions examples/pyo3_benchmarks/tox.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[tox]
# can't install from sdist because local pyo3 repo can't be included in the sdist
skipsdist = true

[testenv]
description = Run the unit tests under {basepython}
deps = -rrequirements-dev.txt
commands =
python setup.py install
pytest {posargs}
5 changes: 2 additions & 3 deletions examples/rustapi_module/tests/test_datetime.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,11 @@ def test_time_fold(fold):
assert t.fold == fold


@pytest.mark.xfail
@pytest.mark.parametrize(
"args", [(-1, 0, 0, 0), (0, -1, 0, 0), (0, 0, -1, 0), (0, 0, 0, -1)]
)
def test_invalid_time_fails_xfail(args):
with pytest.raises(ValueError):
def test_invalid_time_fails_overflow(args):
with pytest.raises(OverflowError):
rdt.make_time(*args)


Expand Down
12 changes: 6 additions & 6 deletions pyo3-macros-backend/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::pymethod::get_arg_names;
use crate::utils;
use proc_macro2::{Span, TokenStream};
use quote::{format_ident, quote};
use syn::{spanned::Spanned, Ident};
use syn::{spanned::Spanned, Ident, Result};

/// Generates the function that is called by the python interpreter to initialize the native
/// module
Expand Down Expand Up @@ -197,7 +197,7 @@ pub fn add_fn_to_module(

let name = &func.sig.ident;
let wrapper_ident = format_ident!("__pyo3_raw_{}", name);
let wrapper = function_c_wrapper(name, &wrapper_ident, &spec, pyfn_attrs.pass_module);
let wrapper = function_c_wrapper(name, &wrapper_ident, &spec, pyfn_attrs.pass_module)?;
Ok(quote! {
#wrapper
pub(crate) fn #function_wrapper_ident<'a>(
Expand All @@ -223,7 +223,7 @@ fn function_c_wrapper(
wrapper_ident: &Ident,
spec: &method::FnSpec<'_>,
pass_module: bool,
) -> TokenStream {
) -> Result<TokenStream> {
let names: Vec<Ident> = get_arg_names(&spec);
let cb;
let slf_module;
Expand All @@ -240,8 +240,8 @@ fn function_c_wrapper(
};
slf_module = None;
};
let body = pymethod::impl_arg_params(spec, None, cb);
quote! {
let body = pymethod::impl_arg_params(spec, None, cb)?;
Ok(quote! {
unsafe extern "C" fn #wrapper_ident(
_slf: *mut pyo3::ffi::PyObject,
_args: *mut pyo3::ffi::PyObject,
Expand All @@ -256,5 +256,5 @@ fn function_c_wrapper(
#body
})
}
}
})
}
Loading