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

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Andrew McCreight [:mccr8]]

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.

Comment 1

[Andrew McCreight [:mccr8]]

Attachment: excerpt from the log

Comment 2

[Andrew McCreight [:mccr8]]

I filed bug 1068285 for figuring out why we have multiple default processes in the first place.

Comment 3

[Andrew McCreight [:mccr8]]

Attachment: If there are multiple bloat views in a single leak log, use the largest one.

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.

Comment 4

[Andrew McCreight [:mccr8]]

try run: https://tbpl.mozilla.org/?tree=Try&rev=ac371ef198ed

Comment 5

[Joel Maher ( :jmaher ) (UTC-4)]

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.

Comment 6

[Andrew McCreight [:mccr8]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/caadbd3acebf

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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