Closed Bug 1264662 Opened 4 years ago Closed 4 years ago

Record IPC message capacity in telemetry

Categories

(Core :: IPC, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
e10s + ---
firefox46 --- wontfix
firefox47 --- fixed
firefox48 --- fixed

People

(Reporter: mccr8, Assigned: mccr8)

References

Details

(Whiteboard: btpp-active)

Attachments

(1 file)

Bug 1260908 added telemetry for recording IPC message size, but we should really be using capacity() instead of size(), so that we capture the full size of the memory allocation. This will also allow telemetry to show the result of IPC message improvements to reduce fragmentation. I don't think there's any reason to record both so I'll just switch what we measure.
Capacity includes internal fragmentation, while size does not.

This requires making capacity() public, but that seems benign.

I only built this locally, but this patch is pretty simple.
Attachment #8741387 - Flags: review?(wmccloskey)
Priority: -- → P1
Whiteboard: btpp-active
Attachment #8741387 - Flags: review?(wmccloskey) → review+
https://hg.mozilla.org/mozilla-central/rev/f7b174cd5ad1
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Comment on attachment 8741387 [details] [diff] [review]
Record IPC message capacity instead of size.

Approval Request Comment
[Feature/regressing bug #]: none
[User impact if declined]: This is needed to make one e10s-related memory measurement more accurate
[Describe test coverage new/current, TreeHerder]: this code runs frequently on TreeHerder, but there is no specific testing
[Risks and why]: low, it just changes one measurement
[String/UUID change made/needed]: none
Attachment #8741387 - Flags: approval-mozilla-aurora?
Hi Andrew, have we verified on Nightly that with this fix the memory measurement now seems correct? I have been requesting Devs to validate telemetry fixes on Nightly channel before requesting Aurora/Beta uplift so as to avoid additional uplifts. Thanks!
Flags: needinfo?(continuation)
We're continuing to get telemetry data for this measurement. The measurement values have stayed the same, or for a few of our common large messages, increased, which is what I'd expect to happen. It is hard to verify anything beyond that from telemetry results.
Flags: needinfo?(continuation)
(In reply to Andrew McCreight [:mccr8] from comment #6)
> We're continuing to get telemetry data for this measurement. The measurement
> values have stayed the same, or for a few of our common large messages,
> increased, which is what I'd expect to happen. It is hard to verify anything
> beyond that from telemetry results.

Thanks Andrew! That should suffice as verification on Nightly. Will approve uplift now.
Comment on attachment 8741387 [details] [diff] [review]
Record IPC message capacity instead of size.

Improving telemetry data quality, Aurora47+
Attachment #8741387 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.