Added warnings to -callgraph-mode and added option for stitched LBR.
#228
Conversation
This file contains 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
Lamoreauxaj
changed the title
callgraph-mode and added option for Stitched_last_branch_record.
Lamoreauxaj
changed the title
callgraph-mode and added option for Stitched_last_branch_record.-callgraph-mode and added option for stitched LBR.
Lamoreauxaj
force-pushed
the
callgraph_mode
branch
from
3a99702
to
b0eb5d8
Compare
Xyene
reviewed
Jun 26, 2022
Xyene
reviewed
Jun 27, 2022
Xyene
reviewed
Jun 27, 2022
Lamoreauxaj
added a commit
to Lamoreauxaj/magic-trace
that referenced
this pull request
Jun 29, 2022
This PR should be merged after janestreet#228 since I referenced that wiki page. And the wiki (and domain) should be updated when this is merged. The only code change is a reference to a wiki page within the `-sampling` flag help message. However the new wiki page draft this is intended to link to can be seen [here](https://github.com/Lamoreauxaj/magic-trace/wiki/Sampling-Backend). Additionally I updated the [timer resolution page](https://github.com/Lamoreauxaj/magic-trace/wiki/Timer-resolution-configuration) to describe changes to configuration. Signed-off-by: Aaron Lamoreaux <alamoreaux@janestreet.com>
This feature has two changes related to `callgraph_mode`. * Based on user feedback from janestreet#224, I added warnings when the `-callgraph-mode` defaults to either `Dwarf` or `Last_branch_record` to notify the user of potential drawbacks from such selection. These warnings only show if they do not explicitly set the flag. * Added a fourth option to run with `Last_branch_record { stitched = true }` which calls `perf report --call-graph lbr` and `perf script --stitch-lbr`. There are a few design decisions here though. First I decided to add it as a variant of `Callgraph_mode` for simplicity of interface for the user. However this has the awkward effect of this now being a parameter needed for both decoding and recording (which `Backend_intf` doesn't support). I would think this could perhaps be useful in other scenarios and so instead of pulling this out to `trace.ml` I decided to add a way for `decode_events` to have access to a `Recording.Data.t` record. Open to alternative ideas though. I also added a wiki page to document the options for `-callgraph-mode`. This is not in the PR, but here is the link to the wiki repo: https://github.com/Lamoreauxaj/magic-trace.wiki.git. See [this page](https://github.com/Lamoreauxaj/magic-trace/wiki/Sampling-callgraph-modes). If this PR is merged, this wiki page would need to be merged too (I referenced it in the cli help documentation). And since I referenced it, the domain would need to redirect to the wiki page. Signed-off-by: Aaron Lamoreaux <alamoreaux@janestreet.com>
Lamoreauxaj
force-pushed
the
callgraph_mode
branch
from
6cb5d30
to
469ac7c
Compare
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.
This feature has two changes related to
callgraph_mode.-callgraph-modedefaults to eitherDwarforLast_branch_recordto notify the user of potential drawbacks from such selection. These warnings only show if they do not explicitly set the flag.Last_branch_record { stitched = true }which callsperf report --call-graph lbrandperf script --stitch-lbr. There are a few design decisions here though. First I decided to add it as a variant ofCallgraph_modefor simplicity of interface for the user. However this has the awkward effect of this now being a parameter needed for both decoding and recording (whichBackend_intfdoesn't support). I would think this could perhaps be useful in other scenarios and so instead of pulling this out totrace.mlI decided to add a way fordecode_eventsto have access to aRecording.Data.trecord. Open to alternative ideas though.I also added a wiki page to document the options for
-callgraph-mode. This is not in the PR, but here is the link to the wiki repo: https://github.com/Lamoreauxaj/magic-trace.wiki.git. See this page. If this PR is merged, this wiki page would need to be merged too (I referenced it in the cli help documentation). And since I referenced it, the domain would need to redirect to the wiki page.