Closed Bug 1068280 Opened 6 years ago Closed 6 years ago

When multiple default processes report a BloatView, the second shadows the first

Categories

(Testing :: Mochitest, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla35

People

(Reporter: mccr8, Assigned: mccr8)

Details

(Whiteboard: [MemShrink:P2])

Attachments

(2 files)

Bug 1067958 was an intermittent leak, but it should have been permaorange.  The logs that are green look like this:

TEST-INFO | leakcheck | threshold set at 5116 bytes
== BloatView: ALL (cumulative) LEAK AND BLOAT STATISTICS, default process 702
... big leak
== BloatView: ALL (cumulative) LEAK AND BLOAT STATISTICS, default process 715
... small leak
...
TEST-INFO | leakcheck | leaked ...various leaked objects for the big leak
TEST-INFO | leakcheck | leaked ...various leaked objects for the small leak
WARNING | leakcheck | 308 bytes leaked (AsyncPanZoomController, Composer2D, CompositorParent, CondVar, CrossProcessMutex, ...)

The list of objects (including AsyncPanZoomController) includes objects from the first big leak, but the size we use to decide if we've exceeded the threshold is from the second leak.

I think both default processes must be dumping stuff into the same log, which is probably a separate bug.  But the leak checking test harness code should notice this and fail in some kind of way.  Or at the very least use the largest leak value it finds.
Whiteboard: [MemShrink]
Attached file excerpt from the log
Attachment #8490338 - Attachment description: except from the log → excerpt from the log
I filed bug 1068285 for figuring out why we have multiple default processes in the first place.
Whiteboard: [MemShrink] → [MemShrink:P2]
Assignee: nobody → continuation
This turned out to be more involved than I'd anticipated.

The main trick is that we record information about the objects in the leak, but we only want to show that for the largest leak, so we need to store that separately and clear it if we find a larger leak.

I made the double bloat view case just warn because it seems to happen with reasonable frequency.  I suspect Nuwa is involved somehow, but I'm not sure.

I take the largest leak rather than the sum of the leaks because the extra 300 bytes or so from the second leak will kick us over the leak threshold.  I don't think we care too much at this point about what these weird phantom other processes are with their tiny leaks.
Attachment #8492439 - Flags: review?(jmaher)
Comment on attachment 8492439 [details] [diff] [review]
If there are multiple bloat views in a single leak log, use the largest one.

Review of attachment 8492439 [details] [diff] [review]:
-----------------------------------------------------------------

this looks reasonable.  Thanks for the comment regarding the bug to split them into individual leak logs per process.  That seems like the right solution.
Attachment #8492439 - Flags: review?(jmaher) → review+
https://hg.mozilla.org/mozilla-central/rev/caadbd3acebf
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.