Start the profiler before XRE_main

RESOLVED WONTFIX

Status

()

Core
Gecko Profiler
RESOLVED WONTFIX
6 years ago
6 years ago

People

(Reporter: BenWa, Unassigned)

Tracking

Trunk
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

6 years ago
Currently we start the profiler in XRE_main for startup profiling. This leaves a blind spot of anything happening before XRE_main such as static initializers. 

Can we move this earlier and how early could be start the profiler? What part of the start-up sequence would we still miss?

Comment 1

6 years ago
I think Taras might have convinced you that this is not worth it.  If not, let me know and I'll try.  :-)
(Reporter)

Comment 2

6 years ago
Here's a summary of this discussion for the purpose of this bug:
taras: BenWa: you cant really go earlier
taras: until you drop dependency on xul.dll 
BenWa: Getting the static initializer should be a low effort hi reward win right?
taras: it'd be interesting
taras: but fairly low reward
BenWa: Ohh, I though we spent a non trivial time doing static constructor
taras: we are
taras: but we know why
BenWa: Well because we're forcing stuff to be paged in I believe, but using wall time we can see which areas are the biggest offenders
taras: it doesn't matter
taras: you page in ~entire binary
taras: cos of random distribution of first-invocation vs layout
taras: so if you dont call one function
taras: you'll call another nearby
taras: at some other time
taras: the solution is to order the binary
taras: (then we could profile to see if the binary order is screwed up)
taras: but i think if you do that
taras: your threads might be suspended
taras: while you page stuff in
taras: i'm not sure how that's handled on os level
(Reporter)

Comment 3

6 years ago
If we're going to be investing into gathering a lot of startup data and getting info into statistic initializer is not much working (moving the init and forcing a linking order) AND the data we collect is accurate and does not skew the data. I see no reason not to do it.

Moving the profiler outside of xul.dll is something I'd like to do but we have more important things to do. I might be worth running a quick test to see how much 'blind spot' we have.

Comment 4

6 years ago
(In reply to comment #3)
> If we're going to be investing into gathering a lot of startup data and getting
> info into statistic initializer is not much working (moving the init and
> forcing a linking order) AND the data we collect is accurate and does not skew
> the data. I see no reason not to do it.

The problem is that mostly the stuff that happens before XRE_main is I/O bound and not CPU-bound.  So, the I/O patterns are interesting.  *But* the I/O patterns will differ based on the layout of our libxul binary, which means that things like PGO on Windows make it practically impossible to apply the knowledge you have gained from your profile to other builds.

Also, note that we have a pretty good idea on what hurts us before we run XRE_main, so I'm not sure what more investigation we need to do there.

Comment 5

6 years ago
It sounds like we've decided not to pursue this, but for whatever it's worth: G++ lets you associate a numeric priority with a constructor; constructors are run in order of increasing priority. Here's the section of the manual:

7.7 C++-Specific Variable, Function, and Type Attributes
========================================================

`init_priority (PRIORITY)'
     In Standard C++, objects defined at namespace scope are guaranteed
     to be initialized in an order in strict accordance with that of
     their definitions _in a given translation unit_.  No guarantee is
     made for initializations across translation units.  However, GNU
     C++ allows users to control the order of initialization of objects
     defined at namespace scope with the `init_priority' attribute by
     specifying a relative PRIORITY, a constant integral expression
     currently bounded between 101 and 65535 inclusive.  Lower numbers
     indicate a higher priority.

     In the following example, `A' would normally be created before
     `B', but the `init_priority' attribute has reversed that order:

          Some_Class  A  __attribute__ ((init_priority (2000)));
          Some_Class  B  __attribute__ ((init_priority (543)));

     Note that the particular values of PRIORITY do not matter; only
     their relative ordering.
(Reporter)

Comment 6

6 years ago
(In reply to Ehsan Akhgari [:ehsan] from comment #4)
> (In reply to comment #3)
> > If we're going to be investing into gathering a lot of startup data and getting
> > info into statistic initializer is not much working (moving the init and
> > forcing a linking order) AND the data we collect is accurate and does not skew
> > the data. I see no reason not to do it.
> 
> The problem is that mostly the stuff that happens before XRE_main is I/O
> bound and not CPU-bound.  So, the I/O patterns are interesting.  *But* the
> I/O patterns will differ based on the layout of our libxul binary, which
> means that things like PGO on Windows make it practically impossible to
> apply the knowledge you have gained from your profile to other builds.
> 
> Also, note that we have a pretty good idea on what hurts us before we run
> XRE_main, so I'm not sure what more investigation we need to do there.

If we're not CPU bound then it means we can collect data without distorting timing. Is there any useful data we could collect such as the I/O patterns (or anything else)? If we can't get any actionable results from collecting this data then I agree it's not worth pursing.
I agree with others, at this point, profiling between xul.dll being loaded and XRE_main being run is not worth the effort. Moving it out of xul.dll in libmozglue would be beneficial, especially on Android, where libmozglue is one of the first things to be loaded, although we don't profile all threads, so nothing really interesting would come up until we do.
(Reporter)

Comment 8

6 years ago
Thanks for the feedback, sounds like we can get more bang for our efforts elsewhere. wont fix at least for now.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.