-
Notifications
You must be signed in to change notification settings - Fork 20
feat: add support for analysis mode #152
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
feat: add support for analysis mode #152
Conversation
CodSpeed Performance ReportMerging #152 will degrade performance by 25.75%Comparing Summary
Benchmarks breakdown
|
5bab4b1 to
f2e74d7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds support for a new "analysis" measurement mode to the CodSpeed benchmarking framework. The analysis mode enables instrumentation similar to simulation mode and integrates with instrument hooks for tracking benchmark execution.
Key changes:
- Introduced a new
Analysisvariant to theMeasurementModeenum - Integrated
InstrumentHooksfor tracking benchmark lifecycle events - Removed the deprecated
is_instrumented()function andRunningOnValgrindenum variant
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/divan_compat/examples/benches/alloc.rs | New benchmark file for testing memory allocation scenarios |
| crates/divan_compat/examples/Cargo.toml | Added the new alloc benchmark to the build configuration |
| crates/codspeed/src/request/mod.rs | Removed unused RunningOnValgrind client request variant |
| crates/codspeed/src/measurement.rs | Removed deprecated is_instrumented() function |
| crates/codspeed/src/codspeed.rs | Integrated InstrumentHooks for benchmark lifecycle tracking |
| crates/cargo-codspeed/src/measurement_mode.rs | Added Analysis variant to measurement modes |
| crates/cargo-codspeed/src/build.rs | Extended codspeed cfg flag to apply to analysis mode |
| .github/workflows/ci.yml | Added CI workflow for testing memory analysis integration |
| crates/codspeed/instrument-hooks | Updated submodule commit reference |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
f2e74d7 to
435a805
Compare
GuillaumeLagrange
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
olgtm
433456e to
274a517
Compare
7eaae17 to
fa4e22b
Compare
GuillaumeLagrange
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
olgtm, although CI outputed the No performance analysis stuff, is this expected with memory mapping not being merged all the way ?
0a30bef to
1fe0cb0
Compare
a0fe3b6 to
4652f6c
Compare
CodSpeed Performance ReportCongrats! CodSpeed is installed 🎉
You will start to see performance impacts in the reports once the benchmarks are run from your default branch.
|
5a3b61f to
3d968aa
Compare
a7bd1c0 to
5a102ae
Compare
e5e19b5 to
baad89c
Compare
df6950a to
031806b
Compare
6020ca6 to
cd1a884
Compare
cd1a884 to
d022e35
Compare
No description provided.