Skip to content

avoid incrementing reference count of function args where possible #5941

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

Merged
davidhewitt merged 6 commits into from
Apr 3, 2026
Merged

avoid incrementing reference count of function args where possible #5941

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
35e02b0
update benchmarks to use borrowed function args
davidhewitt Apr 3, 2026
052a62d
avoid incrementing reference count of function args where possible
davidhewitt Apr 3, 2026
8333e10
clippy + cleanup
davidhewitt Apr 3, 2026
569ee00
newsfragment
davidhewitt Apr 3, 2026
9d0ee04
cleanup, const overflow checks
davidhewitt Apr 3, 2026
44c62ab
Merge remote-tracking branch 'origin/main' into borrowed-func-args
davidhewitt Apr 3, 2026
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
1 change: 1 addition & 0 deletions newsfragments/5941.changed.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Avoid temporary reference count cycles inside `PyAnyMethods::call`, `PyAnyMethods::call1` and `PyAnyMethods::call_method1` for arguments passed as Rust tuples containing borrowed references to Python objects.
38 changes: 18 additions & 20 deletions pyo3-benches/benches/bench_call.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,14 @@ fn bench_call_1(b: &mut Bencher<'_>) {

let foo_module = &module.getattr("foo").unwrap();
let args = (
1.into_pyobject(py).unwrap(),
1i32.into_pyobject(py).unwrap(),
"s".into_pyobject(py).unwrap(),
1.23.into_pyobject(py).unwrap(),
1.23f64.into_pyobject(py).unwrap(),
);

b.iter(|| {
for _ in 0..1000 {
black_box(foo_module).call1(args.clone()).unwrap();
black_box(foo_module).call1(&args).unwrap();
}
});
})
Expand All @@ -52,17 +52,15 @@ fn bench_call(b: &mut Bencher<'_>) {

let foo_module = &module.getattr("foo").unwrap();
let args = (
1.into_pyobject(py).unwrap(),
1i32.into_pyobject(py).unwrap(),
"s".into_pyobject(py).unwrap(),
1.23.into_pyobject(py).unwrap(),
1.23f64.into_pyobject(py).unwrap(),
);
let kwargs = [("d", 1), ("e", 42)].into_py_dict(py).unwrap();

b.iter(|| {
for _ in 0..1000 {
black_box(foo_module)
.call(args.clone(), Some(&kwargs))
.unwrap();
black_box(foo_module).call(&args, Some(&kwargs)).unwrap();
}
});
})
Expand All @@ -77,7 +75,7 @@ fn bench_call_one_arg(b: &mut Bencher<'_>) {

b.iter(|| {
for _ in 0..1000 {
black_box(foo_module).call1((arg.clone(),)).unwrap();
black_box(foo_module).call1((&arg,)).unwrap();
}
});
})
Expand Down Expand Up @@ -116,17 +114,16 @@ class Foo:
);

let foo_module = &module.getattr("Foo").unwrap().call0().unwrap();
let meth = "foo".into_pyobject(py).unwrap();
let args = (
1.into_pyobject(py).unwrap(),
1i32.into_pyobject(py).unwrap(),
"s".into_pyobject(py).unwrap(),
1.23.into_pyobject(py).unwrap(),
1.23f64.into_pyobject(py).unwrap(),
);

b.iter(|| {
for _ in 0..1000 {
black_box(foo_module)
.call_method1("foo", args.clone())
.unwrap();
black_box(foo_module).call_method1(&meth, &args).unwrap();
}
});
})
Expand All @@ -144,17 +141,19 @@ class Foo:
);

let foo_module = &module.getattr("Foo").unwrap().call0().unwrap();
let meth = "foo".into_pyobject(py).unwrap();

let args = (
1.into_pyobject(py).unwrap(),
1i32.into_pyobject(py).unwrap(),
"s".into_pyobject(py).unwrap(),
1.23.into_pyobject(py).unwrap(),
1.23f64.into_pyobject(py).unwrap(),
);
let kwargs = [("d", 1), ("e", 42)].into_py_dict(py).unwrap();

b.iter(|| {
for _ in 0..1000 {
black_box(foo_module)
.call_method("foo", args.clone(), Some(&kwargs))
.call_method(&meth, &args, Some(&kwargs))
.unwrap();
}
});
Expand All @@ -173,13 +172,12 @@ class Foo:
);

let foo_module = &module.getattr("Foo").unwrap().call0().unwrap();
let meth = "foo".into_pyobject(py).unwrap();
let arg = 1i32.into_pyobject(py).unwrap();

b.iter(|| {
for _ in 0..1000 {
black_box(foo_module)
.call_method1("foo", (arg.clone(),))
.unwrap();
black_box(foo_module).call_method1(&meth, (&arg,)).unwrap();
}
});
})
Expand Down
76 changes: 47 additions & 29 deletions src/types/tuple.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,24 @@ use crate::internal_tricks::get_ssize_index;
#[cfg(feature = "experimental-inspect")]
use crate::type_object::PyTypeInfo;
use crate::types::{sequence::PySequenceMethods, PyList, PySequence};
#[cfg(all(
not(any(PyPy, GraalPy)),
any(all(Py_3_9, not(Py_LIMITED_API)), Py_3_12)
))]
use crate::BoundObject;
use crate::{
exceptions, Bound, FromPyObject, IntoPyObject, IntoPyObjectExt, PyAny, PyErr, PyResult, Python,
};
use std::iter::FusedIterator;
#[cfg(feature = "nightly")]
use std::num::NonZero;

#[cfg(all(
not(any(PyPy, GraalPy)),
any(all(Py_3_9, not(Py_LIMITED_API)), Py_3_12)
))]
use libc::size_t;

#[inline]
#[track_caller]
fn try_new_from_iter<'py>(
Expand Down Expand Up @@ -657,14 +668,14 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+
) -> PyResult<Bound<'py, PyAny>> {
let py = function.py();
// We need this to drop the arguments correctly.
let args_bound = [$(self.$n.into_bound_py_any(py)?,)*];
let args_objects = ($(self.$n.into_pyobject_or_pyerr(py)?),*,);
// Prepend one null argument for `PY_VECTORCALL_ARGUMENTS_OFFSET`.
let mut args = [std::ptr::null_mut(), $(args_bound[$n].as_ptr()),*];
let mut args = [std::ptr::null_mut(), $(args_objects.$n.as_ptr()),*];
unsafe {
ffi::PyObject_VectorcallDict(
function.as_ptr(),
args.as_mut_ptr().add(1),
$length + ffi::PY_VECTORCALL_ARGUMENTS_OFFSET,
const { with_vectorcall_arguments_offset($length) },
kwargs.as_ptr(),
)
.assume_owned_or_err(py)
Expand All @@ -678,27 +689,26 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+
_: crate::call::private::Token,
) -> PyResult<Bound<'py, PyAny>> {
let py = function.py();
// We need this to drop the arguments correctly.
let args_bound = [$(self.$n.into_bound_py_any(py)?,)*];
let args_objects = ($(self.$n.into_pyobject_or_pyerr(py)?),*,);

#[cfg(not(Py_LIMITED_API))]
if $length == 1 {
return unsafe {
ffi::PyObject_CallOneArg(
function.as_ptr(),
args_bound[0].as_ptr()
args_objects.0.as_ptr()
)
.assume_owned_or_err(py)
};
}

// Prepend one null argument for `PY_VECTORCALL_ARGUMENTS_OFFSET`.
let mut args = [std::ptr::null_mut(), $(args_bound[$n].as_ptr()),*];
let mut args = [std::ptr::null_mut(), $(args_objects.$n.as_ptr()),*];
unsafe {
ffi::PyObject_Vectorcall(
function.as_ptr(),
args.as_mut_ptr().add(1),
$length + ffi::PY_VECTORCALL_ARGUMENTS_OFFSET,
const { with_vectorcall_arguments_offset($length) },
std::ptr::null_mut(),
)
.assume_owned_or_err(py)
Expand All @@ -713,28 +723,27 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+
_: crate::call::private::Token,
) -> PyResult<Bound<'py, PyAny>> {
let py = object.py();
// We need this to drop the arguments correctly.
let args_bound = [$(self.$n.into_bound_py_any(py)?,)*];
let args_objects = ($(self.$n.into_pyobject_or_pyerr(py)?),*,);

#[cfg(not(Py_LIMITED_API))]
if $length == 1 {
Comment thread
Tpt marked this conversation as resolved.
return unsafe {
ffi::PyObject_CallMethodOneArg(
object.as_ptr(),
method_name.as_ptr(),
args_bound[0].as_ptr(),
object.as_ptr(),
method_name.as_ptr(),
args_objects.0.as_ptr()
)
.assume_owned_or_err(py)
};
}

let mut args = [object.as_ptr(), $(args_bound[$n].as_ptr()),*];
let mut args = [object.as_ptr(), $(args_objects.$n.as_ptr()),*];
unsafe {
ffi::PyObject_VectorcallMethod(
method_name.as_ptr(),
args.as_mut_ptr(),
// +1 for the receiver.
1 + $length + ffi::PY_VECTORCALL_ARGUMENTS_OFFSET,
const { with_vectorcall_arguments_offset(1 + $length) },
std::ptr::null_mut(),
)
.assume_owned_or_err(py)
Expand Down Expand Up @@ -785,15 +794,14 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+
_: crate::call::private::Token,
) -> PyResult<Bound<'py, PyAny>> {
let py = function.py();
// We need this to drop the arguments correctly.
let args_bound = [$(self.$n.into_bound_py_any(py)?,)*];
let args_objects = ($(self.$n.into_pyobject_or_pyerr(py)?),*,);
// Prepend one null argument for `PY_VECTORCALL_ARGUMENTS_OFFSET`.
let mut args = [std::ptr::null_mut(), $(args_bound[$n].as_ptr()),*];
let mut args = [std::ptr::null_mut(), $(args_objects.$n.as_ptr()),*];
unsafe {
ffi::PyObject_VectorcallDict(
function.as_ptr(),
args.as_mut_ptr().add(1),
$length + ffi::PY_VECTORCALL_ARGUMENTS_OFFSET,
const { with_vectorcall_arguments_offset($length) },
kwargs.as_ptr(),
)
.assume_owned_or_err(py)
Expand All @@ -807,27 +815,26 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+
_: crate::call::private::Token,
) -> PyResult<Bound<'py, PyAny>> {
let py = function.py();
// We need this to drop the arguments correctly.
let args_bound = [$(self.$n.into_bound_py_any(py)?,)*];
let args_objects = ($(self.$n.into_pyobject_or_pyerr(py)?),*,);

#[cfg(not(Py_LIMITED_API))]
if $length == 1 {
return unsafe {
ffi::PyObject_CallOneArg(
function.as_ptr(),
args_bound[0].as_ptr()
args_objects.0.as_ptr()
)
.assume_owned_or_err(py)
};
}

// Prepend one null argument for `PY_VECTORCALL_ARGUMENTS_OFFSET`.
let mut args = [std::ptr::null_mut(), $(args_bound[$n].as_ptr()),*];
let mut args = [std::ptr::null_mut(), $(args_objects.$n.as_ptr()),*];
unsafe {
ffi::PyObject_Vectorcall(
function.as_ptr(),
args.as_mut_ptr().add(1),
$length + ffi::PY_VECTORCALL_ARGUMENTS_OFFSET,
const { with_vectorcall_arguments_offset($length) },
std::ptr::null_mut(),
)
.assume_owned_or_err(py)
Expand All @@ -842,28 +849,27 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+
_: crate::call::private::Token,
) -> PyResult<Bound<'py, PyAny>> {
let py = object.py();
// We need this to drop the arguments correctly.
let args_bound = [$(self.$n.into_bound_py_any(py)?,)*];
let args_objects = ($(self.$n.into_pyobject_or_pyerr(py)?),*,);

#[cfg(not(Py_LIMITED_API))]
if $length == 1 {
return unsafe {
ffi::PyObject_CallMethodOneArg(
object.as_ptr(),
method_name.as_ptr(),
args_bound[0].as_ptr(),
args_objects.0.as_ptr(),
)
.assume_owned_or_err(py)
};
}

let mut args = [object.as_ptr(), $(args_bound[$n].as_ptr()),*];
let mut args = [object.as_ptr(), $(args_objects.$n.as_ptr()),*];
unsafe {
ffi::PyObject_VectorcallMethod(
method_name.as_ptr(),
args.as_mut_ptr(),
// +1 for the receiver.
1 + $length + ffi::PY_VECTORCALL_ARGUMENTS_OFFSET,
const { with_vectorcall_arguments_offset(1 + $length) },
std::ptr::null_mut(),
)
.assume_owned_or_err(py)
Expand Down Expand Up @@ -942,6 +948,18 @@ fn array_into_tuple<'py, const N: usize>(
}
}

/// Add `PY_VECTORCALL_ARGUMENTS_OFFSET` to the given number, checking for overflow at compile time.
///
/// Guarantees that we don't accidentally overflow a `size_t` should this get changed in the future.
#[cfg(all(
not(any(PyPy, GraalPy)),
any(all(Py_3_9, not(Py_LIMITED_API)), Py_3_12)
))]
const fn with_vectorcall_arguments_offset(n: size_t) -> size_t {
n.checked_add(ffi::PY_VECTORCALL_ARGUMENTS_OFFSET)
.expect("overflow adding PY_VECTORCALL_ARGUMENTS_OFFSET")
}

tuple_conversion!(1, (ref0, 0, T0));
tuple_conversion!(2, (ref0, 0, T0), (ref1, 1, T1));
tuple_conversion!(3, (ref0, 0, T0), (ref1, 1, T1), (ref2, 2, T2));
Expand Down
Loading