Avoiding struct method call overhead of `extract_pyclass_ref_mut`
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 16.2k
- Forks
- 1k
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 66
Description
Firstly, thanks for PyO3, it's great!
This issue is being opened following a [chat post on gitter].(https://matrix.to/#/!AAhjIWoaKSExrkkhlG:gitter.im/$9gOL75NMgSzKVrKugcQUMBUFcy5QMpLvxuJafAHLieU?via=gitter.im&via=matrix.org&via=nitro.chat)
In short, consider these performance ordered cargo bench results:
running 5 tests
test tests::bench_xoshiro_struct ... bench: 56,930,651 ns/iter (+/- 145,472)
test tests::bench_lcg_struct ... bench: 86,424,781 ns/iter (+/- 337,028)
test tests::bench_lcg_static_lazy ... bench: 86,614,389 ns/iter (+/- 781,944)
test tests::bench_lcg_static ... bench: 87,029,922 ns/iter (+/- 353,495)
test tests::bench_xoshiro_static_lazy ... bench: 108,713,707 ns/iter (+/- 1,500,070)
and contrast that to these timeit results:
lcg_static: 2.2382071590000123
lcg_static_lazy: 2.2475717499983148
xoshiro_static_lazy: 2.333924032998766
lcg_struct: 2.603333091999957
xoshiro_struct: 2.6484997750012553
See something odd there? 🤓 Hey structs! What is going on!?!? Sure we have overhead but all of these test functions are returning a simple type to python and, yeah, the structs have to be mutable but for xoshiro_struct to go from being about twice as fast as the lazy to about 14% slower is unexpected, and to see both of the 'struct' versions leading in the Rust benches to trailing in the Python timeits is unexpected.
Running some long pytest loops on these to get enough samples to see what's going on reveals that our two structs are getting penalized for being structs:
The question is can this be avoided through some hint/annotation to short-circuit that extract/type-info stack?
Here's one of the minimal structs:
#[pyclass]
pub struct XoshiroStruct {
state: Xoshiro256Plus,
}
impl XoshiroStruct {
fn next_state(&mut self) -> u64 {
self.state.next_u64()
}
}
#[pymethods]
impl XoshiroStruct {
#[new]
pub fn new() -> Self {
Self {
state: Xoshiro256Plus::from_entropy(),
}
}
pub fn do_something(&mut self) -> u64 {
do_it(self.next_state())
}
}
Here's the profile: pytest 2024-02-15 22.59 profile.json.gz
And here's a repo with these examples: https://github.com/Thell/struct_perf
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the minimal XoshiroStruct example and the linked struct_perf repository, then inspect the pytest profile and compare its results with the listed cargo bench and timeit results. Determine whether the extract_pyclass_ref_mut and type-info path can be avoided for these mutable methods. Done means a justified implementation or documented limitation, supported by comparable benchmarks.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python, rust
- Domain
- backend-api-design, performance
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100