Closed
Bug 1264662
Opened 9 years ago
Closed 9 years ago
Record IPC message capacity in telemetry
Categories
(Core :: IPC, defect, P1)
Core
IPC
Tracking
()
RESOLVED
FIXED
mozilla48
People
(Reporter: mccr8, Assigned: mccr8)
References
Details
(Whiteboard: btpp-active)
Attachments
(1 file)
(deleted),
patch
|
billm
:
review+
ritu
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•9 years ago
|
||
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.
Assignee | ||
Updated•9 years ago
|
Attachment #8741387 -
Flags: review?(wmccloskey)
Updated•9 years ago
|
Priority: -- → P1
Updated•9 years ago
|
Whiteboard: btpp-active
Attachment #8741387 -
Flags: review?(wmccloskey) → review+
Comment 3•9 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Assignee | ||
Comment 4•9 years ago
|
||
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)
Assignee | ||
Comment 6•9 years ago
|
||
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+
Comment 9•9 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•