Closed Bug 1776931 Opened 3 months ago Closed 2 months ago

Simplify SpiderMonkey telemetry probes

Categories

(Core :: JavaScript Engine, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
104 Branch
Tracking Status
firefox104 --- fixed

People

(Reporter: tcampbell, Assigned: tcampbell)

References

Details

Attachments

(7 files)

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.

Depends on D150653

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

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

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

Depends on D150656

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

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

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

Attachment #9283516 - Attachment description: WIP: Bug 1776931 - Remove unused JSTelemetrySender → Bug 1776931 - Remove unused JSTelemetrySender. r?jonco
Attachment #9283517 - Attachment description: WIP: Bug 1776931 - Unify internal and external JS telemetry names → Bug 1776931 - Unify internal and external JS telemetry names. r?jonco
Attachment #9283518 - Attachment description: WIP: Bug 1776931 - Refactor GC slice telemetry → Bug 1776931 - Refactor GC slice telemetry. r?jonco
Attachment #9283519 - Attachment description: WIP: Bug 1776931 - Add JSMetric enum class for SpiderMonkey telemetry → Bug 1776931 - Add JSMetric enum class for SpiderMonkey telemetry. r?jonco
Attachment #9283520 - Attachment description: WIP: Bug 1776931 - Add js::Metrics struct and replace addTelemetery calls in JS → Bug 1776931 - Add js::Metrics struct and replace addTelemetery calls in JS. r?jandem!,jonco!
Attachment #9283521 - Attachment description: WIP: Bug 1776931 - Use types for SpiderMonkey telemetry → Bug 1776931 - Use types for SpiderMonkey telemetry. r?jandem!,jonco!

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

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

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
Regressions: 1790568
You need to log in before you can comment on or make changes to this bug.