-
Notifications
You must be signed in to change notification settings - Fork 62
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
Implement --unstable-presymbolicate #202
Conversation
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.
Will review more tomorrow.
/// library, so that we can potentially provide symbolication info ahead of time. | ||
/// This is here instead of in `LibraryInfo` because we don't want to serialize it, | ||
/// and because it's currently a hack. | ||
all_libs_seen_rvas: Vec<Option<BTreeSet<u32>>>, |
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.
Please add a comment saying that the Vec is indexed by LibraryHandle.0
.
Alternatively, you could make the Vec indexed by GlobalLibIndex.0
, needing one less translation in add_lib_used_rva
and at that point you could also remove the Option and always initialize it to the empty set.
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.
Yeah, good idea. FWIW, GlobalLibIndex
is a confusing name, maybe just UsedLibraryHandle
?
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.
UsedLibraryHandle
sounds good
samply/src/shared/symbol_precog.rs
Outdated
address: rva, | ||
size: None, |
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.
I'd prefer it if you stored the correct symbol address. Sometimes you have many different addresses in the same function, and we want the front-end to create a shared symbol for those different addresses. Here you're giving them all different symbol addresses, so the front-end will think they're different symbols with the same name.
Ok, implemented proper address/symbol info and properly implemented frames. Should have addressed all the review feedback here. I didn't make the |
Adds
--unstable-presymbolicate
, which will output.syms.json
file alongside the regular profile output. If that file is present when the server is fired up (based on the profile name, i.e.foo.json
->foo.syms.json
), it is parsed and the data is added to be used when symbolicating. If a library has presymbolicated info (by debugid), only that info is used. There's no attempt to fall back to on-disk etc.This allows a capture to be entirely self-contained, so that no symbol servers or on-disk data are needed for looking at the profile, which is very useful for running in CI or other locations where publishing the debuginfo isn't necessarily useful otherwise.