PyO3 / PyO3/pyo3

Avoiding struct method call overhead of `extract_pyclass_ref_mut`

Open
#3,843 4 comments 0 reactions 0 assignees View on GitHub

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:

image

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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.