1505522 - Convert MemoryTelemetry.jsm to C++

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Kris Maglione [:kmag]]

The existing implementation has memory usage and performance issues which are hard to deal with in JS. By migrating to C++, we could make this code much more memory efficient, and also move the expensive data gathering to a background thread to avoid janking the main thread when we collect data.

Comment 1

[Eric Rahm [:erahm]]

Kris, as part of this work can you expand the tests [1] to make sure the values are actually measured? Chris H-C noted anything we gather without expiration should really have a test in bug 1511918, comment 4.

[1] https://searchfox.org/mozilla-central/rev/3fdc51e66c9487b39220ad58dcee275fca070ccd/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js#369

Comment 2

[Kris Maglione [:kmag]]

Attachment: Bug 1505522: Part 1 - Wait for async reports from gatherMemory() to complete when assembling pings. r=chutten

Several call sites assume that gatherMemory() does all of its work
synchronously. This is not true as it is, and will be even less true after my
subsequent patches.

This patch changes gatherMemory() to return a promise which resolves when all
of its in-process work is done, and changes two call sites to wait for it to
resolve before continuing.

One remaining call site, unfortunately, needs to return ping data
synchronously, and therefore cannot wait for async work to complete.

Comment 3

[Kris Maglione [:kmag]]

Attachment: Bug 1505522: Part 2 - Migrate MemoryTelemetry.jsm to C++. r=erahm,chutten

This has benefits both in terms of performance and memory usage. Aside from
the obvious savings of not loading additional JS scripts in every process,
this also allows us to move more of our expensive data collection work to a
background thread, where it doesn't risk janking both parent and content
processes.

MozReview-Commit-ID: 2A593R7bIKB

Comment 4

[Kris Maglione [:kmag]]

Attachment: Bug 1505522: Part 3 - Remove unused getHeapAllocatedAsync method. r=erahm

Comment 5

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3a91b5014f05907245750d28e49316d783e43492
Bug 1505522: Part 1 - Wait for async reports from gatherMemory() to complete when assembling pings. r=chutten

https://hg.mozilla.org/integration/mozilla-inbound/rev/bd8001e1f07efc4b6d512bc2c4e49a9dfd91e1db
Bug 1505522: Part 2 - Migrate MemoryTelemetry.jsm to C++. r=erahm,chutten

https://hg.mozilla.org/integration/mozilla-inbound/rev/fba99cab2299592c3bb7a055aaa320d35bafe5f1
Bug 1505522: Part 3 - Remove unused getHeapAllocatedAsync method. r=erahm

Comment 6

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/3a91b5014f05
https://hg.mozilla.org/mozilla-central/rev/bd8001e1f07e
https://hg.mozilla.org/mozilla-central/rev/fba99cab2299

Comment 7

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c9814f860a6773e3f11eda23aaa8fb9033d77256
Bug 1505522: Follow-up: Resolve gatherMemory() promise in release builds. r=me

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/c9814f860a67