Closed
Bug 1051230
Opened 10 years ago
Closed 7 years ago
Enable leak logging for content processes on desktop
Categories
(Core :: DOM: Content Processes, defect)
Core
DOM: Content Processes
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
e10s | - | --- |
People
(Reporter: billm, Assigned: mccr8)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
(Keywords: meta, Whiteboard: [MemShrink:meta])
Attachments
(1 obsolete file)
It actually works right now, but the content process usually crashes so we never see any leaks.
Assignee | ||
Comment 1•10 years ago
|
||
Thanks, Bill.
Just for some context here, given the plans for a single content process this really needs to work for reals in order to ship e10s. Without it, we'll have no visibility into severe permanent leaks. (If we were using process-per-tab it wouldn't be as big of a deal, because content process leaks would only last until somebody closed a tab, but as it is, these leaks are just as bad as pre-e10s.)
Depends on: 1038943
Updated•10 years ago
|
Assignee | ||
Comment 2•10 years ago
|
||
Apparently bug 1035454 is enough that we start getting child process leak checking. But there are a bunch of leaks, so bug 1052224 is going to disable the leak checking.
Updated•10 years ago
|
Whiteboard: [MemShrink] → [MemShrink:P1]
Assignee | ||
Comment 3•10 years ago
|
||
try run with leak checking enabled: https://tbpl.mozilla.org/?tree=Try&rev=32bf41cea397
(ignore the M3 asserts)
Updated•10 years ago
|
Blocks: old-e10s-m2, e10s-tests
Updated•10 years ago
|
No longer blocks: e10s-tests
Updated•10 years ago
|
Blocks: e10s-harness
Assignee | ||
Comment 4•10 years ago
|
||
Leak logging works once bug 1035454 is landed, so the bigger issue is fixing the content process leaks, so I'm changing this to be about letting us re-enable it. The actual enabling (once bug 1035454 is landed) is basically just going to be backing out bug 1052224.
Summary: Get leak logging working in content processes → Enable leak logging for content processes
Assignee | ||
Comment 5•10 years ago
|
||
I guess I'm working on this the most, so I'll take the bug.
Assignee: nobody → continuation
Does the testharness automatically do sane things with multiple logs?
Assignee | ||
Comment 7•10 years ago
|
||
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #6)
> Does the testharness automatically do sane things with multiple logs?
Yeah, it seems like it. Check out M2 here:
https://tbpl.mozilla.org/?tree=Try&rev=32bf41cea397
Reporter | ||
Comment 8•10 years ago
|
||
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #6)
> Does the testharness automatically do sane things with multiple logs?
It seems to:
http://mxr.mozilla.org/mozilla-central/source/build/automationutils.py#376
Assignee | ||
Comment 9•10 years ago
|
||
This is a hacky work in progress, but it is useful when locally investigating leaks. This patch makes it so that we error when a content child leaks, but only if it is larger than 5kb, which is what we always leak.
Assignee | ||
Comment 10•10 years ago
|
||
On top of my hacky pile of patches, e10s M4 has about 620kb of leaks and M5 has about 200kb. Across a few runs, the size of the former is pretty stables, and the latter was identical in all runs, which is a good sign.
https://tbpl.mozilla.org/?tree=Try&rev=6ce2c7636985
Does the large one include JS? My experience with b2g was that once we got rid of the JS we were leaking the # was extremely stable.
Assignee | ||
Comment 12•10 years ago
|
||
Well, it is only varying by a few hundred bytes, so that's pretty stable.
M2 and M3 hit asserts I added because they are running the GMPService in a content process, so I'm not sure how bad those are going to be once I fix that. I haven't looked at M1 yet.
Assignee | ||
Updated•10 years ago
|
Updated•10 years ago
|
Assignee | ||
Comment 13•10 years ago
|
||
B2G has a number of special issues of its own, so I'm splitting that out into a separate bug, bug 1070068.
Summary: Enable leak logging for content processes → Enable leak logging for content processes on desktop
Assignee | ||
Comment 14•10 years ago
|
||
Comment on attachment 8483012 [details] [diff] [review]
Produce an error for larger child process leaks. WIP
Leak thresholds can now be configured by process type by changing options.leakThresholds, and what processes we error on when they don't produce a log can be adjusted by changing options.ignoreMissingLeaks.
Attachment #8483012 -
Attachment is obsolete: true
Assignee | ||
Comment 15•10 years ago
|
||
e10s M3 has gone from leaking about 11kb in the tab process to leaking 942kb.
Assignee | ||
Comment 16•10 years ago
|
||
Good: https://hg.mozilla.org/try/rev/d7ec36a77534
Bad: https://hg.mozilla.org/try/rev/388e101e75c8
So that's only a window of a few days. I'll try to narrow it down to a test and then a regressing changeset at some point. I tried running the tests that are leaking URLs, but nothing useful came up.
leaking try run: https://tbpl.mozilla.org/?tree=Try&rev=5f123e75dc9c
(I could have also somehow caused the leaks with the patches I added in the try push.)
Assignee | ||
Comment 17•10 years ago
|
||
Bug 1057908 should make us not run the GeckoMediaPluginService in the content process, where it doesn't shut down properly, and thus leaks.
Depends on: 1057908
Assignee | ||
Comment 19•10 years ago
|
||
(In reply to Jim Mathies [:jimm] from comment #18)
> requesting retriage on this tracker.
This has sort of degenerated into a tracking bug, so I'm not sure it needs to be tracked or not. We're running XPCOM leak checking on Linux. Last I checked, it was working on OSX, but we're not actually running it. Windows has a known issue that needs to be fixed, but that work is being tracked in its own bug, bug 1091917.
Assignee | ||
Comment 20•10 years ago
|
||
(We're not running leak checking on OSX, because it only runs in debug builds, and we're not running e10s debug OSX builds on TreeHerder.)
Assignee | ||
Comment 21•9 years ago
|
||
We're doing this testing now, so no need to leave it as a P1.
Whiteboard: [MemShrink:P1] → [MemShrink:meta]
Assignee | ||
Comment 22•7 years ago
|
||
This has been done for a while, so I'll mark it as fixed.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•