892774 - Breakpad Stack scan: speed up MyCodeModules::GetModuleForAddress

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect)

Description

[Benoit Girard (:BenWa)]

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

Comment 1

[Julian Seward [:jseward]]

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.

Comment 2

[Julian Seward [:jseward]]

Attachment: 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).

Comment 3

[Benoit Girard (:BenWa)]

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_.

Comment 4

[Julian Seward [:jseward]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8cc1cd14c1fd

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8cc1cd14c1fd