1466113 - Add realm stats to ZoneGCStats

Status: NEW

(Core :: JavaScript Engine, enhancement, P3)

Description

[Jan de Mooij [:jandem]]

ZoneGCStats has:

* collectedCompartmentCount 
* compartmentCount 
* sweptCompartmentCount

We should either replace these with realm data, or add similar fields for realms.

The compartmentCount is also added to the JSON (as total_compartments); changing the format there might be a bit annoying.

Not very urgent because realms and compartments will be 1:1 in the browser for a while, but we should fix it at some point.

Comment 1

[Jan de Mooij [:jandem]]

Paul, it looks like you touched this stuff fairly recently. If we change a field in the JSON data, what places do we need to update? This affects the Gecko profiler etc right?

Comment 2

[Paul Bone [:pbone]]

Yes.

You can add new fields without any major issue, (but it'd be a good idea to at least document their availability).  But changing or removing fields may cause an issue.

This data is consumed by two things:

 1. Telemetry
 2. Gecko profiler.

Telemetry
=========

Although the data is made available to telemetry, there's nothing in the telemetry service that needs to change other thatn documentation, since it's currently given to the telemetry as a JSON object and then ignored.  I thought there was an external repo but can't find it.  So it might just be: toolkit/components/telemetry/docs/data/main-ping.rst in m-c.

You'll also need to change the browser toolkit components, mostly to change the limits on how many items to send in telemetry messages: toolkit/components/telemetry/GCTelemetry.jsm, these limits are intended to make us aware of when we add more data and use more bandwidth for telemetry.  I numbered each item in the GC code so that it's easy to count and verify how many there are.  Georg has helped with and reviewed my changes to the toolkit component.

Gecko profiler
==============

https://github.com/devtools-html/perf.html

You'll need to update src/types/markers.js to document any changes you make.  And if you remove or change existing fields you'll need to write some conversion code to convert old format profiles into new ones.  That's in src/profile-logic/  If you do that you'll need to bump the version number in m-c also, but I forget where that is.

Depending on the changes you make you may need to land these changes before the ones in m-c so that the pref.html service continues to work.

Let me know if you have any more questions.  I've also been working with Julien and Marcus and they've been very helpful.

Comment 3

[Julien Wajsberg [:julienw]]

A quick note that changes to telemetry data doesn't normally incur changes to the profiler data. But maybe that's how Paul set up the things in the JS engine?

Comment 4

[Jan de Mooij [:jandem]]

Paul, thanks a lot, that's useful information. I'll probably get back to this in a few weeks/months.

(In reply to Julien Wajsberg [:julienw] from comment #3)
> A quick note that changes to telemetry data doesn't normally incur changes
> to the profiler data. But maybe that's how Paul set up the things in the JS
> engine?

See this:

https://github.com/devtools-html/perf.html/blob/4015ef9f0ba53e573fc3f1a27dc1c1ff7dcccde3/src/types/markers.js#L105

Comment 5

[Steve Fink [:sfink] [:s:]]

(In reply to Julien Wajsberg [:julienw] from comment #3)
> A quick note that changes to telemetry data doesn't normally incur changes
> to the profiler data. But maybe that's how Paul set up the things in the JS
> engine?

If I'm following correctly, I would guess that this was from billm, not pbone -- there is special telemetry, specific to the GC, that captures the worst-case and a random sample of the JSON for GCs, for use in detailed analysis. (We also have lots of standard telemetry probes that *are* independent of the profiler.) So I'm guessing what's going on is that the JSON format is reused for the profiler as well as this flight recorder type telemetry stuff.

It landed in bug 1308040 and is accessible at https://analysis.telemetry.mozilla.org/

Comment 6

[Paul Bone [:pbone]]

(In reply to Steve Fink [:sfink] [:s:] (PTO June 31) from comment #5)
> (In reply to Julien Wajsberg [:julienw] from comment #3)
> > A quick note that changes to telemetry data doesn't normally incur changes
> > to the profiler data. But maybe that's how Paul set up the things in the JS
> > engine?
> 
> If I'm following correctly, I would guess that this was from billm, not
> pbone -- there is special telemetry, specific to the GC, that captures the
> worst-case and a random sample of the JSON for GCs, for use in detailed
> analysis. (We also have lots of standard telemetry probes that *are*
> independent of the profiler.) So I'm guessing what's going on is that the
> JSON format is reused for the profiler as well as this flight recorder type
> telemetry stuff.

That's right, When I got here the same JSON formatting code was used by both telemetry and the profiler.  And yeah, the telemetry probes are independent from the telemetry JSON object.  The Telemetry JSON object has a little extra processing in toolkit/components/telemetry/GCTelemetry.jsm which is also where a 2 random and 2 "worst" GC objects are selected and sent to telemetry, the others are discarded.  the profiler also does some extra processing to these JSON objects. but in both cases it's superficial, most of it is the same as what we write from Statistics.cpp and Nursery.cpp (I forgot to mention that file, but I guess you don't need it for realms?).