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.

Sign up for GitHub

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

Jump to bottom

opt: move fastcall boilerplate out of generated code #2085

Merged
merged 1 commit into from Jan 3, 2022
Merged

opt: move fastcall boilerplate out of generated code #2085

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Open in desktop
Expand Up @@ -43,7 +43,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Improve performance and error messages for `#[derive(FromPyObject)]` for enums. [#2068](https://github.com/PyO3/pyo3/pull/2068)
- Reduce generated LLVM code size (to improve compile times) for:
- internal `handle_panic` helper [#2074](https://github.com/PyO3/pyo3/pull/2074)
- `#[pyfunction]` and `#[pymethods]` argument extraction [#2075](https://github.com/PyO3/pyo3/pull/2075)
- `#[pyfunction]` and `#[pymethods]` argument extraction [#2075](https://github.com/PyO3/pyo3/pull/2075) [#2085](https://github.com/PyO3/pyo3/pull/2085)
- `#[pyclass]` type object creation [#2076](https://github.com/PyO3/pyo3/pull/2076) [#2081](https://github.com/PyO3/pyo3/pull/2081)

### Removed
Expand Down
31 changes: 9 additions & 22 deletions pyo3-macros-backend/src/method.rs
View file
Open in desktop
Expand Up @@ -496,7 +496,7 @@ impl<'a> FnSpec<'a> {
}
}
CallingConvention::Fastcall => {
let arg_convert_and_rust_call = impl_arg_params(self, cls, rust_call, &py, true)?;
let arg_convert = impl_arg_params(self, cls, &py, true)?;
quote! {
unsafe extern "C" fn #ident (
_slf: *mut #krate::ffi::PyObject,
Expand All @@ -508,23 +508,14 @@ impl<'a> FnSpec<'a> {
#deprecations
_pyo3::callback::handle_panic(|#py| {
#self_conversion
let _kwnames: ::std::option::Option<&_pyo3::types::PyTuple> = #py.from_borrowed_ptr_or_opt(_kwnames);
// Safety: &PyAny has the same memory layout as `*mut ffi::PyObject`
let _args = _args as *const &_pyo3::PyAny;
let _kwargs = if let ::std::option::Option::Some(kwnames) = _kwnames {
::std::slice::from_raw_parts(_args.offset(_nargs), kwnames.len())
} else {
&[]
};
let _args = ::std::slice::from_raw_parts(_args, _nargs as usize);

#arg_convert_and_rust_call
#arg_convert
#rust_call
})
}
}
}
CallingConvention::Varargs => {
let arg_convert_and_rust_call = impl_arg_params(self, cls, rust_call, &py, false)?;
let arg_convert = impl_arg_params(self, cls, &py, false)?;
quote! {
unsafe extern "C" fn #ident (
_slf: *mut #krate::ffi::PyObject,
Expand All @@ -535,17 +526,15 @@ impl<'a> FnSpec<'a> {
#deprecations
_pyo3::callback::handle_panic(|#py| {
#self_conversion
let _args = #py.from_borrowed_ptr::<_pyo3::types::PyTuple>(_args);
let _kwargs: ::std::option::Option<&_pyo3::types::PyDict> = #py.from_borrowed_ptr_or_opt(_kwargs);

#arg_convert_and_rust_call
#arg_convert
#rust_call
})
}
}
}
CallingConvention::TpNew => {
let rust_call = quote! { #rust_name(#(#arg_names),*) };
let arg_convert_and_rust_call = impl_arg_params(self, cls, rust_call, &py, false)?;
let arg_convert = impl_arg_params(self, cls, &py, false)?;
quote! {
unsafe extern "C" fn #ident (
subtype: *mut #krate::ffi::PyTypeObject,
Expand All @@ -556,10 +545,8 @@ impl<'a> FnSpec<'a> {
#deprecations
use _pyo3::callback::IntoPyCallbackOutput;
_pyo3::callback::handle_panic(|#py| {
let _args = #py.from_borrowed_ptr::<_pyo3::types::PyTuple>(_args);
let _kwargs: ::std::option::Option<&_pyo3::types::PyDict> = #py.from_borrowed_ptr_or_opt(_kwargs);

let result = #arg_convert_and_rust_call;
#arg_convert
let result = #rust_call;
let initializer: _pyo3::PyClassInitializer::<#cls> = result.convert(#py)?;
let cell = initializer.create_cell_from_subtype(#py, subtype)?;
::std::result::Result::Ok(cell as *mut _pyo3::ffi::PyObject)
Expand Down
62 changes: 28 additions & 34 deletions pyo3-macros-backend/src/params.rs
View file
Open in desktop
Expand Up @@ -53,12 +53,11 @@ fn is_kwargs(attrs: &[Argument], name: &syn::Ident) -> bool {
pub fn impl_arg_params(
spec: &FnSpec<'_>,
self_: Option<&syn::Type>,
body: TokenStream,
py: &syn::Ident,
fastcall: bool,
) -> Result<TokenStream> {
if spec.args.is_empty() {
return Ok(body);
return Ok(TokenStream::new());
}

let args_array = syn::Ident::new("output", Span::call_site());
Expand All @@ -70,11 +69,11 @@ pub fn impl_arg_params(
for (i, arg) in spec.args.iter().enumerate() {
arg_convert.push(impl_arg_param(arg, spec, i, None, &mut 0, py, &args_array)?);
}
return Ok(quote! {{
let _args = Some(_args);
return Ok(quote! {
let _args = ::std::option::Option::Some(#py.from_borrowed_ptr::<_pyo3::types::PyTuple>(_args));
let _kwargs: ::std::option::Option<&_pyo3::types::PyDict> = #py.from_borrowed_ptr_or_opt(_kwargs);
#(#arg_convert)*
#body
}});
});
};

let mut positional_parameter_names = Vec::new();
Expand All @@ -95,7 +94,7 @@ pub fn impl_arg_params(

if kwonly {
keyword_only_parameters.push(quote! {
_pyo3::derive_utils::KeywordOnlyParameterDescription {
_pyo3::impl_::extract_argument::KeywordOnlyParameterDescription {
name: #name,
required: #required,
}
Expand Down Expand Up @@ -142,28 +141,30 @@ pub fn impl_arg_params(
};
let python_name = &spec.python_name;

let (args_to_extract, kwargs_to_extract) = if fastcall {
// _args is a &[&PyAny], _kwnames is a Option<&PyTuple> containing the
// keyword names of the keyword args in _kwargs
(
// need copied() for &&PyAny -> &PyAny
quote! { ::std::iter::Iterator::copied(_args.iter()) },
quote! { _kwnames.map(|kwnames| {
use ::std::iter::Iterator;
kwnames.as_slice().iter().copied().zip(_kwargs.iter().copied())
}) },
)
let extract_expression = if fastcall {
quote! {
DESCRIPTION.extract_arguments_fastcall(
#py,
_args,
_nargs,
_kwnames,
&mut #args_array
)?
}
} else {
// _args is a &PyTuple, _kwargs is an Option<&PyDict>
(
quote! { _args.iter() },
quote! { _kwargs.map(|dict| dict.iter()) },
)
quote! {
DESCRIPTION.extract_arguments_tuple_dict(
#py,
_args,
_kwargs,
&mut #args_array
)?
}
};

// create array of arguments, and then parse
Ok(quote! {{
const DESCRIPTION: _pyo3::derive_utils::FunctionDescription = _pyo3::derive_utils::FunctionDescription {
Ok(quote! {
const DESCRIPTION: _pyo3::impl_::extract_argument::FunctionDescription = _pyo3::impl_::extract_argument::FunctionDescription {
cls_name: #cls_name,
func_name: stringify!(#python_name),
positional_parameter_names: &[#(#positional_parameter_names),*],
Expand All @@ -175,17 +176,10 @@ pub fn impl_arg_params(
};

let mut #args_array = [::std::option::Option::None; #num_params];
let (_args, _kwargs) = DESCRIPTION.extract_arguments(
#py,
#args_to_extract,
#kwargs_to_extract,
&mut #args_array
)?;
let (_args, _kwargs) = #extract_expression;

#(#param_conversion)*

#body
}})
})
}

/// Re option_pos: The option slice doesn't contain the py: Python argument, so the argument
Expand Down