Closed Bug 892774 Opened 8 years ago Closed 8 years ago
Breakpad Stack scan: speed up My
Code Modules::Get Module For Address
According to this profile by jld: https://people.mozilla.com/~bgirard/cleopatra/#report=2af1780356d123e3e2c91102441e2e48c0d95611&search=breakpad We don't us/ report stack scanning currently and it's really hurting us (We might later).
Stack scanning is slow because the basic action is to take words off the stack and see if they point just after call instructions. To do that safely (without segfaults) requires calling MyCodeModules::GetModuleForAddress for each word. The implementation is stupid in that it searches sequentially though the known modules. Patch to follow.
Patch that adds two levels of optimisation to MyCodeModules::GetModuleFromAddress: * binary search for the module instead of sequential search * cache min/max code addresses, so as to throw out obviously-out-of-range requests without binary search Testing on x86_64-linux, this reduces the number of tested MyCodeModules per call from about 65 to about 5, viz, a factor of more than 10 improvement. I also discovered that around 55% of the calls to GetModuleFromAddress find no module, presumably because the stack scanner is feeding it values which are either small integers, stack addresses or heap addresses (that it finds on the stack). Patch is defensively written. It carefully audits what it gets from SharedLibraryInfo::GetInfoForSelf, so as to ensure that the binary search cannot get into infinite loops even if GetInfoForSelf gives us garbage results (out of order or overlapping address ranges).
Attachment #775321 - Flags: review?(bgirard)
Summary: Breakpad Stack scanning is slowing down unwinds → Breakpad Stack scan: speed up MyCodeModules::GetModuleForAddress
Assignee: nobody → jseward
Comment on attachment 775321 [details] [diff] [review] Patch, v1 Review of attachment 775321 [details] [diff] [review]: ----------------------------------------------------------------- ::: tools/profiler/UnwinderThread2.cpp @@ +1705,3 @@ > } > + > + // Binary search in |mods_|. lo and hi need to be signed, else You could use http://www.cplusplus.com/reference/algorithm/binary_search/ with the iterator of mods_.
Attachment #775321 - Flags: review?(bgirard) → review+
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.