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.

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

Generic parameters on Bencher are not API compatible with criterion #13

Closed
davidhewitt opened this issue Sep 6, 2023 · 3 comments · Fixed by #22
Closed

Generic parameters on Bencher are not API compatible with criterion #13

davidhewitt opened this issue Sep 6, 2023 · 3 comments · Fixed by #22

Comments

@davidhewitt
Copy link
Contributor

In PyO3 we tend to manifest all lifetime parameters in structures, eg: https://github.com/PyO3/pyo3/blob/8dc3d2bc118bd03d206c666bf0ad1d304b737728/pyo3-benches/benches/bench_any.rs#L64

(So we write &mut Bencher<'_> instead of &mut Bencher.)

The compat mode of this crate doesn't have the lifetime parameter (or the generic parameter M: Measurement, but that's not relevant to us in PyO3).

I think for the lifetime it would be possible to solve this by changing the definition of Bencher to be

pub struct Bencher<'a> {
    codspeed: &'a mut CodSpeed,
    uri: String,
}
@art049
Copy link
Member

art049 commented Sep 12, 2023

I didn't think about this while building the adapter but it makes complete sense. I'll work on it ASAP.

@art049
Copy link
Member

art049 commented Oct 20, 2023

Just released it in 2.3.0, let me know if that works for you!

@davidhewitt
Copy link
Contributor Author

Thanks, yep I've merged the codspeed benchmarks into PyO3 this morning, look forward to seeing how it works out!

marc2332 added a commit to marc2332/codspeed-rust that referenced this issue Feb 10, 2024
refactor: `Slider` now manages its value via state
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants