Closed
Bug 809692
Opened 12 years ago
Closed 12 years ago
Start the profiler before XRE_main
Categories
(Core :: Gecko Profiler, defect)
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: BenWa, Unassigned)
Details
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•12 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•12 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•12 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•12 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•12 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•12 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.
Comment 7•12 years ago
|
||
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•12 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
Closed: 12 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•