etw-stackwalk: send debug GUIDs from browser [requires perfetto change] [chromium/src : main]
Sean Maher (Gerrit)
Sean Maher added 3 comments![]( "Open in Gerrit")
std::unordered_map<std::string, std::string> image_guids_;if this is more than like 8 items large i think we should use a more performant map, say absl::flat_hash_map or whatever it's called
L"advapi32.dll"};in system32: ntoskrnl.exe, dbghelp.dll, user32.dll, apphelp.dll
in chrome: libEGL.dll, libGLESv2.dll, dxil.dll, dxcompiler.dll, vulkan-1.dll, maybe also the other ones
// ⭐⭐⭐ TODO: do we also need age? it's needed to pull symbolsare you typing these stars
Related details
- Jesse McKenna
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sean Maher (Gerrit)
Sean Maher added 1 comment![]( "Open in Gerrit")
image_guids.emplace(entry.key(), entry.value());keep in mind that these are gonna be NT kernel pathes (start with `\HardDisk0\` or whatedver)
Related details
- Jesse McKenna
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jesse McKenna (Gerrit)
Jesse McKenna voted and added 5 comments![]( "Open in Gerrit")
Votes added by Jesse McKenna
| Commit-Queue | +1 |
5 comments
Thank you for the comments! I've also cleaned up this CL, but there's a large point I'm still unsure on:
etiennep@ mentioned that the method in my earlier patchsets wouldn't allow the debug IDs to be added when recording from the UI. I'm not sure if this refers to `ui.perfetto.dev` or somewhere else - I've moved the logic to `AdaptPerfettoConfigForChrome`, but this is also not called when tracing from `ui.perfetto.dev`, so if we do want to make sure debug IDs are sent when recording from there, I think I'll need to move it somewhere more generic.
`ConsumerHost::EnableTracing` seems like it applies to all tracing entry points, but that class seems fairly generic so it feels awkward to pass arbitrary data from it. Let me know if you or Etienne have any thoughts. I thought it'd be worth sending this for review for now anyway so we can discuss. Thanks again!
std::unordered_map<std::string, std::string> image_guids_;if this is more than like 8 items large i think we should use a more performant map, say absl::flat_hash_map or whatever it's called
Done
image_guids.emplace(entry.key(), entry.value());keep in mind that these are gonna be NT kernel pathes (start with `\HardDisk0\` or whatedver)
Done - I had the `HandleImageLoadEvent` code convert them (via `base::DevicePathToDriveLetterPath`) in https://chromium-review.git.corp.google.com/c/chromium/src/+/7794349
in system32: ntoskrnl.exe, dbghelp.dll, user32.dll, apphelp.dll
in chrome: libEGL.dll, libGLESv2.dll, dxil.dll, dxcompiler.dll, vulkan-1.dll, maybe also the other ones
Done - this required doing some fallback logic because some of these DLLs won't necessarily be loaded (e.g., the graphics ones aren't loaded in the browser AFAICT).
// ⭐⭐⭐ TODO: do we also need age? it's needed to pull symbolsare you typing these stars
Yeah, I use them to highlight places I need to pay attention to, but they're not intended to be part of the final draft - removed in the latest : )
Related details
- Etienne Pierre-Doray
- Sean Maher
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Etienne Pierre-Doray (Gerrit)
Etienne Pierre-Doray added 4 comments![]( "Open in Gerrit")
Thank you for the comments! I've also cleaned up this CL, but there's a large point I'm still unsure on:
etiennep@ mentioned that the method in my earlier patchsets wouldn't allow the debug IDs to be added when recording from the UI. I'm not sure if this refers to `ui.perfetto.dev` or somewhere else - I've moved the logic to `AdaptPerfettoConfigForChrome`, but this is also not called when tracing from `ui.perfetto.dev`, so if we do want to make sure debug IDs are sent when recording from there, I think I'll need to move it somewhere more generic.
`ConsumerHost::EnableTracing` seems like it applies to all tracing entry points, but that class seems fairly generic so it feels awkward to pass arbitrary data from it. Let me know if you or Etienne have any thoughts. I thought it'd be worth sending this for review for now anyway so we can discuss. Thanks again!
Ya I don't think doing this in AdaptPerfettoConfigForChrome would work if recording from perfetto (but there isn't an easy way to enable etw from perfetto anyways).
I think it'd be fine to do this in ConsumerHost::EnableTracing though.
absl::flat_hash_map<std::string, std::string> known_debug_ids);const flat_hash_map& ?
GetStackSamplingDebugIds() {Regarless of where we do AdaptEtwConfig, I think this should be defined in components/tracing/common/
auto as_debug_id = [](const GUID& guid, DWORD age) {
auto debug_id = base::win::WStringFromGUID(guid);
base::RemoveChars(debug_id, L"{}-", &debug_id);
debug_id.append(base::NumberToWString(age));
return base::WideToUTF8(debug_id);
};Could we reuse (or rather extract) logic from GetDebugInfoForModule()?
Related details
- Jesse McKenna
- Sean Maher
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |