Skip to content

Commit

Permalink
opt: move fastcall boilerplate out of generated code
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhewitt committed Jan 3, 2022
1 parent 2503a2d commit 8116002
Show file tree
Hide file tree
Showing 6 changed files with 554 additions and 412 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Loading

0 comments on commit 8116002

Please sign in to comment.