1667272 - Record stacks of when services are loaded

Status: RESOLVED FIXED

(Toolkit :: Startup and Profile System, defect)

Description

[Nick Alexander :nalexander [he/him]]

Bug 1398198 captures JS stacks when JS components and modules are loaded. I am digging into details of when various XPCOM services are loaded very early in startup, and wanted to record stacks to help me understand this.

So I added this, and learned some things, including that capturing native stacks isn't particularly useful. But having done it I thought I would post it and see if we might land it: perhaps it can be made more useful (by adding optional JS stacks, or getting mixed stacks like the profiler/BHR).

Comment 1

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1667272 - Pre: Support `MOZ_BROWSER_STARTUP_RECORD` environment variable equivalent to `browser.startup.record` preference.

Comment 2

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1667272 - Record (native) stacks when services are instantiated under `browser.startup.record`.

DO NOT LAND: This functionality is slow; it should be put behind a
second environment variable and pref pair.

Depends on D91371

Comment 3

[Florian Quèze [:florian]]

Attachment: Add profiler markers for service loads

(In reply to Nick Alexander :nalexander [he/him] from comment #0)

So I added this, and learned some things, including that capturing native stacks isn't particularly useful. But having done it I thought I would post it and see if we might land it: perhaps it can be made more useful (by adding optional JS stacks, or getting mixed stacks like the profiler/BHR).

Have you considered adding profiler markers? Here is a startup profile on my local build with the attached patch, which adds GetService markers a some more label frames: https://share.firefox.dev/342LWNr

Comment 4

[Florian Quèze [:florian]]

FWIW, I worked on bug 1398198 several years ago, and given how much the profiler has progressed since that, if I worked on the browser_startup.js test today, I would likely remove most of the startup recorder, and instead make the test read markers from a startup profile (similar to the https://searchfox.org/mozilla-central/source/browser/base/content/test/performance/browser_startup_mainthreadio.js test that I wrote more recently, and that has code to extract markers from the JSON data of a profile, and read their stacks).

Comment 5

[Nick Alexander :nalexander [he/him]]

Florian -- thanks for your thoughts in this area. I have kinda-sorta considered using profiler markers and rejected it because of the unusual use case that I have. We're investigating a non-standard startup path that would be used to run certain JS code from the command line as a background task, and in that context it's not clear to me if/how much of the profiler would be available.

But... having pursued this patch as a learning exercise, and having found out why capturing these stacks wasn't done in the first place, I'll abandon this particular technical expression in favour of thinking through the role of the profiler in our use case.

Thanks again, Florian!

Comment 6

[Florian Quèze [:florian]]

Is the profiler hard to use in your case? For cases where using the profiler UI is difficult, what usually works is using startup+shutdown profiling. This dumps a JSON file to the disk, and we can them symbolicate it either using python scripts (we do this for mochitests and talos profiles) or when loading the profile into profiler.firefox.com (eg. dropping the file on the page) in the same local build.

Should I request review for my patch in this bug? Or open a follow-up?

Comment 7

[Florian Quèze [:florian]]

After discussing with Nick, we concluded the profiler will be usable for his use case, so I'll request review of my patch here.

Comment 8

[Florian Quèze [:florian]]

Attachment: Bug 1667272 - Add profiler markers when Services are loaded, r=mstange.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/24e00ef863bf
Add profiler markers when Services are loaded, r=mstange.

Comment 10

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/24e00ef863bf

Comment 11

[Nick Alexander :nalexander [he/him]]

Clearing NI since this has landed. Thanks, Florian!