Deduplicate walltime perf symbols, unwind data and debug info across pids#240
Merged
GuillaumeLagrange merged 4 commits intomainfrom Feb 20, 2026
Merged
Conversation
99e0ea4 to
1a32faa
Compare
15316f4 to
9305a03
Compare
a3617af to
7f65690
Compare
not-matthias
requested changes
Feb 12, 2026
Member
not-matthias
left a comment
There was a problem hiding this comment.
I think the struct naming can be clarified a bit, I was struggling quite a lot to understand how they map together. Maybe also using custom types for the key that's used in the hashmaps.
The overall logic looks good! Next round should be quick
...apshots/codspeed_runner__executor__wall_time__perf__unwind_data__tests__cpp_unwind_data.snap
Outdated
Show resolved
Hide resolved
192d2ce to
14cb24f
Compare
14cb24f to
9a7f9bc
Compare
not-matthias
approved these changes
Feb 13, 2026
Member
not-matthias
left a comment
There was a problem hiding this comment.
LGTM overall. Just a few minor stylistic comments.
e43f420 to
f308463
Compare
31d5b35 to
642e4fd
Compare
This fixes a regression introduced in 8b37208. We now filter pids using `bench_pids`, except for `exec-harness` integrations, where we take all pids.
- Store pid-agnostic data in a file or json map under a mapped `path_key` for each elf - For each pid, store pid specific data, mostly the computed load_bias from where each module was loaded into memory at runtime, alongside a key to retrieve the pid-agnostic data This way, we only write to disk relevant parts of the information.
Also add a rebuild trigger to make it easier to run GITHUB_ACTIONS=1 cargo test` locally. We could have a better trigger, but this will do for now.
642e4fd to
82a55b9
Compare
54f94be to
5bb49e6
Compare
art049
approved these changes
Feb 17, 2026
Member
art049
left a comment
There was a problem hiding this comment.
Just the naming thing TBD
Otherwise, lgtm! this needs extensive testing before release though
68ca926 to
82a55b9
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.
Merge/deployment
Content
Since the real deduplicator source of truth is actually the path of the elf file on disk, I settled on
it allows for the filename to be determined by the actual path, without having to either nest all the files, or have to sanitize paths to file name and end up in awkward file-name length limitations
Here's what it looks like on a 1000 iterations report of one simple program