Manage valgrind instrumentation with profiling cli#209
Merged
GuillaumeLagrange merged 8 commits intomainfrom Jan 23, 2026
Merged
Conversation
084f925 to
c58ae98
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds support for managing Valgrind instrumentation in exec-harness by introducing a C preload library that enables/disables callgrind instrumentation. The library is compiled during build, embedded into the binary, and loaded via LD_PRELOAD when running benchmarks in simulation mode.
Changes:
- Added C preload library for Valgrind instrumentation control
- Implemented build script to compile and embed the preload library
- Added simulation/instrumentation mode support with valgrind-based execution
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/exec-harness/build.rs | New build script that compiles the C preload library and exports constants |
| crates/exec-harness/preload/codspeed_preload.c | New C library that manages Valgrind instrumentation via constructor/destructor |
| crates/exec-harness/src/constants.rs | New module defining shared constants between Rust and C code |
| crates/exec-harness/src/analysis.rs | Added perform_with_valgrind function that extracts and uses the preload library |
| crates/exec-harness/src/lib.rs | Enabled simulation mode with "instrumentation" alias |
| crates/exec-harness/src/uri.rs | Added command field and print_executing helper method |
| crates/exec-harness/src/walltime/mod.rs | Added logging for benchmark execution |
| crates/exec-harness/Cargo.toml | Updated codspeed to 4.2.0, added build dependencies |
| .gitignore | Added compile_commands.json and .cache |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…umentation For now, the library is just generated and its path is passed to the binary through env file. In the next commit, this will change to embed the binary in the final exec-harness executable to make the built artifact self-sufficient.
0559320 to
617472b
Compare
Also switch to tempfile
617472b to
4bee3df
Compare
not-matthias
requested changes
Jan 22, 2026
44d172f to
e29c367
Compare
not-matthias
approved these changes
Jan 22, 2026
9d47d79 to
c341c75
Compare
cdb155f to
a90b8ff
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
constructoranddestructorattributes (which are supported by bothgccandclang)LD_PRELOAD, after writing it to disk at runtime