Simplify SpiderMonkey telemetry probes
Categories
(Core :: JavaScript Engine, enhancement, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox104 | --- | fixed |
People
(Reporter: tcampbell, Assigned: tcampbell)
References
(Blocks 1 open bug)
Details
Attachments
(7 files)
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review |
Cleanup how we define SpiderMonkey telemetry to bring us more in line with how both Glean and V8 organize metrics. To keep things simple, use C macro lists now and defer the migration to Glean FoG until a later time (because build system.. sigh).
In this proposal:
- SpiderMonkey will continue to do internal accumulation in some cases
- XPCJSRuntime will still use hooks to send data out to the Firefox telemetry system
- Use
rt->metrics()
as the object to manipulate telemetry on. V8 usesisolate->metrics()
pretty successfully. - The
Histograms.json
/geckoview/streaming/metrics.yaml
files still duplicate the probe definition in SpiderMonkey. This is similar in Chrome/V8. - Metric types are introduced to SpiderMonkey with rough relations to Glean metrics. These are more for documentation purposes at this point and the measurement code for things like TimeDuration remains the same as before.
- Synchronize internal probe names with firefox names. It was confusing when the mismatched and I prefer a comment at the location of collection to explain the name if needed. This lets us drop boilerplate code.
- I want to replace / redefine MAP_JS_TELEMETRY which is in the public API. It is a pretty easy thing for embedders to adapt to and I doubt anyone even uses the telemetry API.
Assignee | ||
Comment 1•7 months ago
|
||
Depends on D150653
Assignee | ||
Comment 2•7 months ago
|
||
Update JS telemetry names to match the corresponding Firefox probe names to
avoid any confusion. We usually updated these internal names with _2 suffixes
were added, but not always.
Depends on D150654
Assignee | ||
Comment 3•7 months ago
|
||
Use TimeDuration more directly in the GC slice telemetry statistics. This
prepares for later patches that add my type safety to telemetry. Measured values
are unchanged.
Depends on D150655
Assignee | ||
Comment 4•7 months ago
|
||
Replace the JS_TELEMETRY_* enums with a JSMetric enum class. Use this to
generate the XPCJSRuntime mapping automatically.
Depends on D150656
Assignee | ||
Comment 5•7 months ago
|
||
This brings us closer to how both Glean FoG and V8 handle telemetry and lets us
start having better type checking later.
Depends on D150657
Assignee | ||
Comment 6•7 months ago
|
||
Similar to Glean FoG, use types for telemetry metrics. For our existing probes,
this is mainly about using TimeDuration directly. The reported values continue
to be unchanged.
Depends on D150658
Assignee | ||
Comment 7•7 months ago
|
||
This is probably ready for some feedback. Next, I'll prototype more (non-gc) probes and see what the patches look like
Updated•7 months ago
|
Updated•7 months ago
|
Updated•7 months ago
|
Updated•7 months ago
|
Updated•7 months ago
|
Updated•7 months ago
|
Assignee | ||
Comment 8•7 months ago
|
||
Example of adding telemetry probe in new system. This example is bad for
performance though since we have many functions and this triggers a binary
search within buckets for each sample.
Depends on D150659
Assignee | ||
Comment 9•7 months ago
|
||
This cleans up the existing metrics, but still has some ergonomic issues for android if we don't support Glean.
Comment 10•7 months ago
|
||
Pushed by tcampbell@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/03ae147acb8e Remove unused JSTelemetrySender. r=jonco https://hg.mozilla.org/integration/autoland/rev/02aa9fab8100 Unify internal and external JS telemetry names. r=jonco https://hg.mozilla.org/integration/autoland/rev/cfd08837bd54 Refactor GC slice telemetry. r=jonco https://hg.mozilla.org/integration/autoland/rev/4e7228a6f694 Add JSMetric enum class for SpiderMonkey telemetry. r=jonco https://hg.mozilla.org/integration/autoland/rev/7cdbe9719782 Add js::Metrics struct and replace addTelemetery calls in JS. r=jandem,jonco https://hg.mozilla.org/integration/autoland/rev/4667a0889dd0 Use types for SpiderMonkey telemetry. r=jandem,jonco
Comment 11•7 months ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/03ae147acb8e
https://hg.mozilla.org/mozilla-central/rev/02aa9fab8100
https://hg.mozilla.org/mozilla-central/rev/cfd08837bd54
https://hg.mozilla.org/mozilla-central/rev/4e7228a6f694
https://hg.mozilla.org/mozilla-central/rev/7cdbe9719782
https://hg.mozilla.org/mozilla-central/rev/4667a0889dd0
Updated•2 months ago
|
Description
•