1776931 - Simplify SpiderMonkey telemetry probes

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P2)

Description

[Ted Campbell [:tcampbell]]

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 uses isolate->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.

Comment 1

[Ted Campbell [:tcampbell]]

Attachment: Bug 1776931 - Remove unused JSTelemetrySender. r?jonco

Depends on D150653

Comment 2

[Ted Campbell [:tcampbell]]

Attachment: Bug 1776931 - Unify internal and external JS telemetry names. r?jonco

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

Comment 3

[Ted Campbell [:tcampbell]]

Attachment: Bug 1776931 - Refactor GC slice telemetry. r?jonco

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

Comment 4

[Ted Campbell [:tcampbell]]

Attachment: Bug 1776931 - Add JSMetric enum class for SpiderMonkey telemetry. r?jonco

Replace the JS_TELEMETRY_* enums with a JSMetric enum class. Use this to
generate the XPCJSRuntime mapping automatically.

Depends on D150656

Comment 5

[Ted Campbell [:tcampbell]]

Attachment: Bug 1776931 - Add js::Metrics struct and replace addTelemetery calls in JS. r?jandem!,jonco!

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

Comment 6

[Ted Campbell [:tcampbell]]

Attachment: Bug 1776931 - Use types for SpiderMonkey telemetry. r?jandem!,jonco!

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

Comment 7

[Ted Campbell [:tcampbell]]

This is probably ready for some feedback. Next, I'll prototype more (non-gc) probes and see what the patches look like

Comment 8

[Ted Campbell [:tcampbell]]

Attachment: WIP: Bug 1776931 - DO_NOT_LAND Add telemetry for JS function bytecode size

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

Comment 9

[Ted Campbell [:tcampbell]]

This cleans up the existing metrics, but still has some ergonomic issues for android if we don't support Glean.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
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

[Atila Butkovits]

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