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

introduce trampolines for methods #2705

Merged
merged 6 commits into from
Oct 25, 2022
Merged

introduce trampolines for methods #2705

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8a9b25a
introduce trampolines for methods
davidhewitt Jun 19, 2022
dab0f2c
macro-generated dealloc trampoline
davidhewitt Oct 23, 2022
d1517bd
ensure trampolines use PanicTrap
davidhewitt Oct 23, 2022
2972587
trampoline_inner for getter and setter
davidhewitt Oct 23, 2022
2f25361
allow clippy annotation on dealloc temporary wrapper
davidhewitt Oct 23, 2022
a3f59c8
kill off panic_result_into_callback_output
davidhewitt Oct 23, 2022
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
1 change: 1 addition & 0 deletions newsfragments/2705.changed.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Change `#[pyfunction]` and `#[pymethods]` to use a common call "trampoline" to slightly reduce generated code size and compile times.
122 changes: 77 additions & 45 deletions pyo3-macros-backend/src/method.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,81 +481,68 @@ impl<'a> FnSpec<'a> {
CallingConvention::Noargs => {
let call = rust_call(vec![]);
quote! {
unsafe extern "C" fn #ident (
unsafe fn #ident<'py>(
#py: _pyo3::Python<'py>,
_slf: *mut _pyo3::ffi::PyObject,
_args: *mut _pyo3::ffi::PyObject,
) -> *mut _pyo3::ffi::PyObject
{
) -> _pyo3::PyResult<*mut _pyo3::ffi::PyObject> {
#deprecations
let gil = _pyo3::GILPool::new();
let #py = gil.python();
_pyo3::callback::panic_result_into_callback_output(#py, ::std::panic::catch_unwind(move || -> _pyo3::PyResult<_> {
#self_conversion
#call
}))
#self_conversion
#call
}
}
}
CallingConvention::Fastcall => {
let (arg_convert, args) = impl_arg_params(self, cls, &py, true)?;
let call = rust_call(args);
quote! {
unsafe extern "C" fn #ident (
unsafe fn #ident<'py>(
#py: _pyo3::Python<'py>,
_slf: *mut _pyo3::ffi::PyObject,
_args: *const *mut _pyo3::ffi::PyObject,
_nargs: _pyo3::ffi::Py_ssize_t,
_kwnames: *mut _pyo3::ffi::PyObject) -> *mut _pyo3::ffi::PyObject
{
_kwnames: *mut _pyo3::ffi::PyObject
) -> _pyo3::PyResult<*mut _pyo3::ffi::PyObject> {
#deprecations
let gil = _pyo3::GILPool::new();
let #py = gil.python();
_pyo3::callback::panic_result_into_callback_output(#py, ::std::panic::catch_unwind(move || -> _pyo3::PyResult<_> {
#self_conversion
#arg_convert
#call
}))
#self_conversion
#arg_convert
#call
}
}
}
CallingConvention::Varargs => {
let (arg_convert, args) = impl_arg_params(self, cls, &py, false)?;
let call = rust_call(args);
quote! {
unsafe extern "C" fn #ident (
unsafe fn #ident<'py>(
#py: _pyo3::Python<'py>,
_slf: *mut _pyo3::ffi::PyObject,
_args: *mut _pyo3::ffi::PyObject,
_kwargs: *mut _pyo3::ffi::PyObject) -> *mut _pyo3::ffi::PyObject
{
_kwargs: *mut _pyo3::ffi::PyObject
) -> _pyo3::PyResult<*mut _pyo3::ffi::PyObject> {
#deprecations
let gil = _pyo3::GILPool::new();
let #py = gil.python();
_pyo3::callback::panic_result_into_callback_output(#py, ::std::panic::catch_unwind(move || -> _pyo3::PyResult<_> {
#self_conversion
#arg_convert
#call
}))
#self_conversion
#arg_convert
#call
}
}
}
CallingConvention::TpNew => {
let (arg_convert, args) = impl_arg_params(self, cls, &py, false)?;
let call = quote! { #rust_name(#(#args),*) };
quote! {
unsafe extern "C" fn #ident (
unsafe fn #ident(
#py: _pyo3::Python<'_>,
subtype: *mut _pyo3::ffi::PyTypeObject,
_args: *mut _pyo3::ffi::PyObject,
_kwargs: *mut _pyo3::ffi::PyObject) -> *mut _pyo3::ffi::PyObject
{
_kwargs: *mut _pyo3::ffi::PyObject
) -> _pyo3::PyResult<*mut _pyo3::ffi::PyObject> {
use _pyo3::callback::IntoPyCallbackOutput;
#deprecations
use _pyo3::{callback::IntoPyCallbackOutput, pyclass_init::PyObjectInit};
let gil = _pyo3::GILPool::new();
let #py = gil.python();
_pyo3::callback::panic_result_into_callback_output(#py, ::std::panic::catch_unwind(move || -> _pyo3::PyResult<_> {
#arg_convert
let result = #call;
let initializer: _pyo3::PyClassInitializer::<#cls> = result.convert(#py)?;
initializer.into_new_object(#py, subtype)
}))
#arg_convert
let result = #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 All @@ -571,21 +558,66 @@ impl<'a> FnSpec<'a> {
CallingConvention::Noargs => quote! {
_pyo3::impl_::pymethods::PyMethodDef::noargs(
#python_name,
_pyo3::impl_::pymethods::PyCFunction(#wrapper),
_pyo3::impl_::pymethods::PyCFunction({
unsafe extern "C" fn trampoline(
Copy link
Member

Choose a reason for hiding this comment

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

Would be cool not to have to repeat the (almost) identical signature here and in the trampoline fn. But the required macro trickery isn't worth it, likely?

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree with this, however I'd like to defer it to a follow-up PR sometime (there's a fair bit of refactoring which could improve the macro code, and I'd like it not to distract readers of this PR from the behavioural change).

_slf: *mut _pyo3::ffi::PyObject,
_args: *mut _pyo3::ffi::PyObject,
) -> *mut _pyo3::ffi::PyObject
{
_pyo3::impl_::trampoline::noargs(
_slf,
_args,
#wrapper
)
}
trampoline
}),
#doc,
)
},
CallingConvention::Fastcall => quote! {
_pyo3::impl_::pymethods::PyMethodDef::fastcall_cfunction_with_keywords(
#python_name,
_pyo3::impl_::pymethods::PyCFunctionFastWithKeywords(#wrapper),
_pyo3::impl_::pymethods::PyCFunctionFastWithKeywords({
unsafe extern "C" fn trampoline(
_slf: *mut _pyo3::ffi::PyObject,
_args: *const *mut _pyo3::ffi::PyObject,
_nargs: _pyo3::ffi::Py_ssize_t,
_kwnames: *mut _pyo3::ffi::PyObject
) -> *mut _pyo3::ffi::PyObject
{
_pyo3::impl_::trampoline::fastcall_with_keywords(
_slf,
_args,
_nargs,
_kwnames,
#wrapper
)
}
trampoline
}),
#doc,
)
},
CallingConvention::Varargs => quote! {
_pyo3::impl_::pymethods::PyMethodDef::cfunction_with_keywords(
#python_name,
_pyo3::impl_::pymethods::PyCFunctionWithKeywords(#wrapper),
_pyo3::impl_::pymethods::PyCFunctionWithKeywords({
unsafe extern "C" fn trampoline(
_slf: *mut _pyo3::ffi::PyObject,
_args: *mut _pyo3::ffi::PyObject,
_kwargs: *mut _pyo3::ffi::PyObject,
) -> *mut _pyo3::ffi::PyObject
{
_pyo3::impl_::trampoline::cfunction_with_keywords(
_slf,
_args,
_kwargs,
#wrapper
)
}
trampoline
}),
#doc,
)
},
Expand Down
143 changes: 102 additions & 41 deletions pyo3-macros-backend/src/pymethod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,22 @@ fn impl_py_method_def_new(cls: &syn::Type, spec: &FnSpec<'_>) -> Result<MethodAn
let slot_def = quote! {
_pyo3::ffi::PyType_Slot {
slot: _pyo3::ffi::Py_tp_new,
pfunc: #cls::#wrapper_ident as _pyo3::ffi::newfunc as _
pfunc: {
unsafe extern "C" fn trampoline(
subtype: *mut _pyo3::ffi::PyTypeObject,
args: *mut _pyo3::ffi::PyObject,
kwargs: *mut _pyo3::ffi::PyObject,
) -> *mut _pyo3::ffi::PyObject
{
_pyo3::impl_::trampoline::newfunc(
subtype,
args,
kwargs,
#cls::#wrapper_ident
)
}
trampoline
} as _pyo3::ffi::newfunc as _
}
};
Ok(MethodAndSlotDef {
Expand All @@ -321,7 +336,22 @@ fn impl_call_slot(cls: &syn::Type, mut spec: FnSpec<'_>) -> Result<MethodAndSlot
let slot_def = quote! {
_pyo3::ffi::PyType_Slot {
slot: _pyo3::ffi::Py_tp_call,
pfunc: #cls::#wrapper_ident as _pyo3::ffi::ternaryfunc as _
pfunc: {
unsafe extern "C" fn trampoline(
slf: *mut _pyo3::ffi::PyObject,
args: *mut _pyo3::ffi::PyObject,
kwargs: *mut _pyo3::ffi::PyObject,
) -> *mut _pyo3::ffi::PyObject
{
_pyo3::impl_::trampoline::ternaryfunc(
slf,
args,
kwargs,
#cls::#wrapper_ident
)
}
trampoline
} as _pyo3::ffi::ternaryfunc as _
}
};
Ok(MethodAndSlotDef {
Expand Down Expand Up @@ -484,24 +514,20 @@ pub fn impl_py_setter_def(
};

let associated_method = quote! {
unsafe extern "C" fn #wrapper_ident(
unsafe fn #wrapper_ident(
_py: _pyo3::Python<'_>,
_slf: *mut _pyo3::ffi::PyObject,
_value: *mut _pyo3::ffi::PyObject,
_: *mut ::std::os::raw::c_void
) -> ::std::os::raw::c_int {
let gil = _pyo3::GILPool::new();
let _py = gil.python();
_pyo3::callback::panic_result_into_callback_output(_py, ::std::panic::catch_unwind(move || -> _pyo3::PyResult<_> {
#slf
let _value = _py
.from_borrowed_ptr_or_opt(_value)
.ok_or_else(|| {
_pyo3::exceptions::PyAttributeError::new_err("can't delete attribute")
})?;
let _val = _pyo3::FromPyObject::extract(_value)?;

_pyo3::callback::convert(_py, #setter_impl)
}))
) -> _pyo3::PyResult<::std::os::raw::c_int> {
#slf
let _value = _py
.from_borrowed_ptr_or_opt(_value)
.ok_or_else(|| {
_pyo3::exceptions::PyAttributeError::new_err("can't delete attribute")
})?;
let _val = _pyo3::FromPyObject::extract(_value)?;

_pyo3::callback::convert(_py, #setter_impl)
}
};

Expand All @@ -510,7 +536,22 @@ pub fn impl_py_setter_def(
#deprecations
_pyo3::class::PySetterDef::new(
#python_name,
_pyo3::impl_::pymethods::PySetter(#cls::#wrapper_ident),
_pyo3::impl_::pymethods::PySetter({
unsafe extern "C" fn trampoline(
slf: *mut _pyo3::ffi::PyObject,
value: *mut _pyo3::ffi::PyObject,
closure: *mut ::std::os::raw::c_void,
) -> ::std::os::raw::c_int
{
_pyo3::impl_::trampoline::setter(
slf,
value,
closure,
#cls::#wrapper_ident
)
}
trampoline
}),
#doc
)
})
Expand Down Expand Up @@ -555,7 +596,6 @@ pub fn impl_py_getter_def(
..
} => {
// named struct field
//quote!(_slf.#ident.clone())
quote!(::std::clone::Clone::clone(&(_slf.#ident)))
}
PropertyType::Descriptor { field_index, .. } => {
Expand Down Expand Up @@ -608,17 +648,13 @@ pub fn impl_py_getter_def(
};

let associated_method = quote! {
unsafe extern "C" fn #wrapper_ident(
_slf: *mut _pyo3::ffi::PyObject,
_: *mut ::std::os::raw::c_void
) -> *mut _pyo3::ffi::PyObject {
let gil = _pyo3::GILPool::new();
let _py = gil.python();
_pyo3::callback::panic_result_into_callback_output(_py, ::std::panic::catch_unwind(move || -> _pyo3::PyResult<_> {
#slf
let item = #getter_impl;
#conversion
}))
unsafe fn #wrapper_ident(
_py: _pyo3::Python<'_>,
_slf: *mut _pyo3::ffi::PyObject
) -> _pyo3::PyResult<*mut _pyo3::ffi::PyObject> {
#slf
let item = #getter_impl;
#conversion
}
};

Expand All @@ -627,7 +663,20 @@ pub fn impl_py_getter_def(
#deprecations
_pyo3::class::PyGetterDef::new(
#python_name,
_pyo3::impl_::pymethods::PyGetter(#cls::#wrapper_ident),
_pyo3::impl_::pymethods::PyGetter({
unsafe extern "C" fn trampoline(
slf: *mut _pyo3::ffi::PyObject,
closure: *mut ::std::os::raw::c_void,
) -> *mut _pyo3::ffi::PyObject
{
_pyo3::impl_::trampoline::getter(
slf,
closure,
#cls::#wrapper_ident
)
}
trampoline
}),
#doc
)
})
Expand Down Expand Up @@ -1050,21 +1099,33 @@ impl SlotDef {
return_mode.as_ref(),
)?;
let associated_method = quote! {
unsafe extern "C" fn #wrapper_ident(_raw_slf: *mut _pyo3::ffi::PyObject, #(#arg_idents: #arg_types),*) -> #ret_ty {
unsafe fn #wrapper_ident(
#py: _pyo3::Python<'_>,
_raw_slf: *mut _pyo3::ffi::PyObject,
#(#arg_idents: #arg_types),*
) -> _pyo3::PyResult<#ret_ty> {
let _slf = _raw_slf;
let gil = _pyo3::GILPool::new();
let #py = gil.python();
_pyo3::callback::panic_result_into_callback_output(#py, ::std::panic::catch_unwind(move || -> _pyo3::PyResult<_> {
#body
}))
#body
}
};
let slot_def = quote! {
let slot_def = quote! {{
unsafe extern "C" fn trampoline(
_slf: *mut _pyo3::ffi::PyObject,
#(#arg_idents: #arg_types),*
) -> #ret_ty
{
_pyo3::impl_::trampoline:: #func_ty (
_slf,
#(#arg_idents,)*
#cls::#wrapper_ident
)
}

_pyo3::ffi::PyType_Slot {
slot: _pyo3::ffi::#slot,
pfunc: #cls::#wrapper_ident as _pyo3::ffi::#func_ty as _
pfunc: trampoline as _pyo3::ffi::#func_ty as _
}
};
}};
Ok(MethodAndSlotDef {
associated_method,
slot_def,
Expand Down