Breakpad Stack scan: speed up MyCodeModules::GetModuleForAddress

RESOLVED FIXED in mozilla26

Status

()

Core
Gecko Profiler
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: BenWa, Assigned: jseward)

Tracking

Trunk
mozilla26
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
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).
(Assignee)

Comment 1

4 years ago
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.
(Assignee)

Comment 2

4 years ago
Created attachment 775321 [details] [diff] [review]
Patch, v1

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).
(Assignee)

Updated

4 years ago
Attachment #775321 - Flags: review?(bgirard)
(Assignee)

Updated

4 years ago
Summary: Breakpad Stack scanning is slowing down unwinds → Breakpad Stack scan: speed up MyCodeModules::GetModuleForAddress
(Assignee)

Updated

4 years ago
Assignee: nobody → jseward
(Reporter)

Comment 3

4 years ago
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+
(Assignee)

Comment 4

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/8cc1cd14c1fd
https://hg.mozilla.org/mozilla-central/rev/8cc1cd14c1fd
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.