Closed Bug 1620097 Opened 5 years ago Closed 5 years ago

No observer notification markers for category observers (e.g. GeckoViewStartup.js profile-after-change)

Categories

(Core :: Gecko Profiler, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla76
Tracking Status
firefox76 --- fixed

People

(Reporter: mstange, Assigned: mstange)

Details

Attachments

(1 file)

Profile: https://perfht.ml/2PLNapZ

In the selected part of this profile, JavaScript code is entered through an observe method on GeckoViewStartup. However, there is no marker for the observer notification, and the observer topic is not indicated on the stack. That's because this is not a regular nsObserverService notification, but instead it's a notification that's dispatched from NS_CreateServicesFromCategory. Category observers are registered in a manifest file, for example GeckoViewStartup is registered for app-startup and profile-after-change.

I saw in some other profiles you shared (https://perfht.ml/2TsFH19) that you have at least a work in progress patch. It's nice to see this information in the marker chart and in the call tree, thanks!

Some other feedback:

  • The current marker name (eg. [topic: profile-after-change] [entry: GeckoViewStartup]) is less readable than it could be. Here are 2 alternative suggestions: profile-after-change (GeckoViewStartup) or profile-after-change: GeckoViewStartup. I think it's important to be as concise as we can because in the marker chart we often see only the beginning of the name.
  • The label frames could similarly be shortened. Eg. Category observer notification [topic: profile-after-change] [entry: URLDecorationAnnotationsService] could be Category observer notification — profile-after-change: URLDecorationAnnotationsService.
  • We miss a marker when creating JS XPCOM components from the category observer notifications. This is the NS_CreateServicesFromCategory calls in the profile. When they create a service implemented in a JS component, we should have a JS XPCOM marker in the marker chart, which would help to understand which pieces of JS code we are loading. I don't know why they aren't covered by the existing JS XPCOM markers.
  • In the call tree, the frames above the 'Category observer notification' label frames are different for the samples that contain NS_CreateServicesFromCategory and for the samples that call the observer (the latter miss the XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&), XREMain::XRE_mainRun() and nsXREDirProvider::DoStartup() stack frames). This makes the call tree confusing.

Some of these points might be better addressed in their own bugs, but I figured I would share what I see in your profiles, hope this helps.

Thanks, this is super helpful feedback! I updated the patch with the format Category observer notification — profile-after-change (URLDecorationAnnotationsService).

(In reply to Florian Quèze [:florian] from comment #1)

  • We miss a marker when creating JS XPCOM components from the category observer notifications. This is the NS_CreateServicesFromCategory calls in the profile. When they create a service implemented in a JS component, we should have a JS XPCOM marker in the marker chart, which would help to understand which pieces of JS code we are loading. I don't know why they aren't covered by the existing JS XPCOM markers.

Can you give some examples? I'm not very familiar with JS component loading. But here's what I can see:

  • There is a JS XPCOM marker for GeckoViewStartup.js before the category observer notification to GeckoViewStartup: https://perfht.ml/38vvDJd
  • There is ChromeUtils.import marker for CrashService.jsm before the category observer notification to CrashService: https://perfht.ml/38ybzFQ
  • In the call tree, the frames above the 'Category observer notification' label frames are different for the samples that contain NS_CreateServicesFromCategory and for the samples that call the observer (the latter miss the XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&), XREMain::XRE_mainRun() and nsXREDirProvider::DoStartup() stack frames). This makes the call tree confusing.

It looks like this is a mix of bug 1529108 and bug 1434402.

(In reply to Markus Stange [:mstange] from comment #2)

(In reply to Florian Quèze [:florian] from comment #1)

  • We miss a marker when creating JS XPCOM components from the category observer notifications. This is the NS_CreateServicesFromCategory calls in the profile. When they create a service implemented in a JS component, we should have a JS XPCOM marker in the marker chart, which would help to understand which pieces of JS code we are loading. I don't know why they aren't covered by the existing JS XPCOM markers.

Can you give some examples? I'm not very familiar with JS component loading. But here's what I can see:

  • There is a JS XPCOM marker for GeckoViewStartup.js before the category observer notification to GeckoViewStartup: https://perfht.ml/38vvDJd
  • There is ChromeUtils.import marker for CrashService.jsm before the category observer notification to CrashService: https://perfht.ml/38ybzFQ

Forget this point, I was just confused by the mix of JS XPCOM and ChromeUtils.import markers. I just verified and in this profile all the category markers have a matching JS XPCOM or ChromeUtils.import marker, except for one that seems to be implemented in C++. Sorry for the noise.

Pushed by mstange@themasta.com: https://hg.mozilla.org/integration/autoland/rev/615af48fd355 Add markers and labels for category service observer notifications. r=florian
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla76
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: