Closed
Bug 1061202
Opened 10 years ago
Closed 10 years ago
Increased memory use window-objects top(none)/detached, leaks the world
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
Tracking | Status | |
---|---|---|
firefox33 | --- | unaffected |
firefox34 | + | fixed |
firefox35 | + | fixed |
b2g-v2.0 | --- | unaffected |
b2g-v2.0M | --- | unaffected |
b2g-v2.1 | --- | fixed |
b2g-v2.2 | --- | fixed |
People
(Reporter: jmjjeffery, Assigned: bzbarsky)
References
Details
(Keywords: memory-leak, regression, Whiteboard: [MemShrink:P1])
Attachments
(8 files, 1 obsolete file)
431.42 KB,
application/x-gzip
|
Details | |
3.94 MB,
application/x-7z-compressed
|
Details | |
5.29 MB,
application/x-7z-compressed
|
Details | |
100.40 KB,
application/gzip
|
Details | |
99.01 KB,
application/gzip
|
Details | |
3.86 KB,
text/plain
|
Details | |
159 bytes,
text/html
|
Details | |
3.02 KB,
patch
|
khuey
:
review+
bajaj
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Noticed a significant increase in memory use using my normal set of tabs where after a few hours mem use will balloon to a gig or more, where it normally on my system runs 500-600meg.
good: 20140829122301 9b3fbc370631
bad: 20140829123201 2a354048f964
421.67 MB (100.0%) -- explicit
├──256.32 MB (60.79%) -- window-objects
│ ├───85.32 MB (20.23%) -- top(none)/detached
│ │ ├──70.68 MB (16.76%) ++ (67 tiny)
│ │ ├──10.15 MB (02.41%) -- window(http://forums.mozillazine.org/viewforum.php?f=23)
│ │ │ ├───9.98 MB (02.37%) -- js-compartment(http://forums.mozillazine.org/viewforum.php?f=23, about:blank)
│ │ │ │ ├──8.36 MB (01.98%) ++ classes
│ │ │ │ └──1.62 MB (00.38%) ++ (4 tiny)
│ │ │ └───0.17 MB (00.04%) ++ (3 tiny)
Note large amount of windows-objects and the large number of (tiny) those (tiny) are mostly link to google ads
STR: Load the 'bad' build as noted above:
Open in tabs several mozillazine forums
http://forums.mozillazine.org/viewforum.php?f=23
http://forums.mozillazine.org/viewforum.php?f=38
http://forums.mozillazine.org/viewforum.php?f=7
I have a total of six open all the time.
Repeat several times over the tabs a 'reload' of page. The more its reloaded/refreshed you will note that in the Windows Taskmanager that memory use is growing and is not reduced over time/reloads.
Tested on win7 x64, I have the 'new cache' disabled, but I've not tested with it enabled.
Over time as the memory use increases the browser becomes sluggish.
Opening 'about:memory' will cause the browser to go 'not responding' for a brief period before recovering and loading the data.
Need help finding the regression from m-i builds.
Your regression range consists of a b2g-inbound merge, which seems unlikely.
If you could provide the verbose CC/GC logs we could investigate those. They may contain private information, so if you don't want to post them on the bug email me @mozilla.com and we'll figure something out.
Reporter | ||
Comment 2•10 years ago
|
||
Files are too big to upload, even zipped one was over 34meg
I don't have a dropbox account - suggestions ?
Andrew, can you make arrangements to get the files from Jim and analyze them?
Flags: needinfo?(continuation)
Comment 4•10 years ago
|
||
Jim, if you could create the logs earlier when you start seeing something leaking?
Comment 5•10 years ago
|
||
Can you reproduce in safe mode? It could be an addon.
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #3)
> Andrew, can you make arrangements to get the files from Jim and analyze them?
Sure.
Jim, like Olli said, if you could get a log from when you first start seeing top(none)/detached window-objects that could be useful, as the logs won't be quite so large, but should still show the problem.
It would be quite handy to have an addon that periodically checks for ghost-windows. :)
Flags: needinfo?(continuation)
Reporter | ||
Comment 6•10 years ago
|
||
Updated•10 years ago
|
Flags: needinfo?(continuation)
Reporter | ||
Comment 7•10 years ago
|
||
Sorry, I'm not having any luck getting the gc.log verbose below 10meg :(
Reporter | ||
Comment 8•10 years ago
|
||
Attachment #8482363 -
Attachment is obsolete: true
Reporter | ||
Comment 9•10 years ago
|
||
Reporter | ||
Comment 10•10 years ago
|
||
after retesting the potential regression range, I still stand by my post above.
It does not take too many reloads of the tabs to force memory use as monitored in the taskmanager to 600meg +, where with the good build it stays around my normal average for this tab-set I use at around 420meg.
Comment 11•10 years ago
|
||
I've noticed a whole lot of ghost windows today.
Will attach a memory reports from before and after minimise.
Comment 12•10 years ago
|
||
Comment 13•10 years ago
|
||
Comment 14•10 years ago
|
||
I can reproduce this by visiting http://online.wsj.com/articles/why-i-just-cant-become-chinese-1409333549.
This is a regression from bug 1011738, which added a strong edge from the observer service to documents at https://hg.mozilla.org/mozilla-central/rev/7c682fb2f547#l2.13. Commenting that out makes this go away.
This makes nightly basically unusable. We need a fix ASAP.
Assignee: nobody → fabrice
Blocks: 1011738
blocking-b2g: --- → 2.1+
status-firefox33:
--- → unaffected
status-firefox34:
--- → affected
status-firefox35:
--- → affected
tracking-firefox34:
--- → ?
tracking-firefox35:
--- → ?
Keywords: regressionwindow-wanted
(In reply to Jim Jeffery not reading bug-mail 1/2/11 from comment #10)
> after retesting the potential regression range, I still stand by my post
> above.
Yep, you were correct.
Updated•10 years ago
|
Flags: needinfo?(continuation)
Keywords: mlk
OS: Windows 7 → All
Hardware: x86_64 → All
Whiteboard: [MemShrink]
Assignee | ||
Comment 18•10 years ago
|
||
Hrm. Are we ending up with documents That we OnPageShow but never OnPageHide?
I'll look into why tomorrow, on the link from comment 15... Because we really do need some "this document is not being rendered anymore" notification, and I was pretty sure OnPageHide was it.
It appears so, yes. Commenting out the AddObserver call makes the leak go away, so RemoveObserver is clearly not getting called.
Updated•10 years ago
|
Comment 20•10 years ago
|
||
bz, any progress? This is bad...
Flags: needinfo?(bzbarsky)
Summary: Increased memory use window-objects top(none)/detached → Increased memory use window-objects top(none)/detached, leaks the world
Whiteboard: [MemShrink] → [MemShrink:P1]
Assignee | ||
Comment 21•10 years ago
|
||
We end up calling OnPageShow twice on the same document (once for the about:blank, once for the document.close()) without an intermediate OnPageShow. That adds us to the observer service twice, but only removes once later, so we leak.
Sadly this doesn't show up as a shutdown leak for some reason. :(
Flags: needinfo?(bzbarsky)
Its not a shutdown leak because the observer service dumps all the observers at shutdown.
Assignee | ||
Comment 23•10 years ago
|
||
This seems to make things all better for me
Attachment #8483248 -
Flags: review?(khuey)
Assignee | ||
Updated•10 years ago
|
Assignee: fabrice → bzbarsky
Status: NEW → ASSIGNED
Assignee | ||
Comment 24•10 years ago
|
||
> Its not a shutdown leak because the observer service dumps all the observers at shutdown.
Gah. :(
Comment on attachment 8483248 [details] [diff] [review]
Make sure to not double-add app-theme-changed observers when a document has OnPageShow called on it twice without an OnPageHide call in between
Review of attachment 8483248 [details] [diff] [review]:
-----------------------------------------------------------------
Assert that we're not observing in the dtor?
r=me
Attachment #8483248 -
Flags: review?(khuey) → review+
Assignee | ||
Comment 26•10 years ago
|
||
> Assert that we're not observing in the dtor?
Ah, that would catch it at shutdown time, good point.
Assignee | ||
Comment 27•10 years ago
|
||
Added:
MOZ_ASSERT(!mObservingAppThemeChanged,
"Document leaked to shutdown, then the observer service dropped "
"its ref to us so we were able to go away.");
in ~nsDocument.
Assignee | ||
Comment 28•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Target Milestone: --- → mozilla35
Reporter | ||
Comment 29•10 years ago
|
||
(In reply to Boris Zbarsky [:bz] from comment #28)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/483f0a6a540e
Testing with this m-i build seems to have fixed the leaks for me, things are now running normal here as far as memory use goes.
Assignee | ||
Comment 30•10 years ago
|
||
Jim, thank you for testing that, and for reporting and narrowing down this thing!
Assignee | ||
Comment 31•10 years ago
|
||
Comment on attachment 8483248 [details] [diff] [review]
Make sure to not double-add app-theme-changed observers when a document has OnPageShow called on it twice without an OnPageHide call in between
Approval Request Comment
[Feature/regressing bug #]: Bug 1011738
[User impact if declined]: Nasty leaks.
[Describe test coverage new/current, TBPL]: We added some asserts that should
help catch issues if any arise....
[Risks and why]: Risk should be quite low.
[String/UUID change made/needed]: None.
Attachment #8483248 -
Flags: approval-mozilla-aurora?
Comment 32•10 years ago
|
||
Backed out for causing mochitest-bc crashes on multiple platforms.
https://hg.mozilla.org/integration/mozilla-inbound/rev/75211a0eded5
https://tbpl.mozilla.org/php/getParsedLog.php?id=47319846&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=47317771&tree=Mozilla-Inbound
Assignee | ||
Comment 33•10 years ago
|
||
Looks like mochitest-bc in fact leaks documents to shutdown, triggering the assert I added in ~nsDocument. It's also triggering, in the same function:
###!!! ASSERTION: Destroying a currently-showing document: '!mIsShowing', file /builds/slave/m-in-osx64-d-00000000000000000/build/content/base/src/nsDocument.cpp, line 1602
which is even worse...
I'll downgrade to an NS_ASSERTION from a MOZ_ASSERT for now so mochitest-bc can keep ignoring the fact that it's leaking shit. :(
Assignee | ||
Comment 34•10 years ago
|
||
I filed bug 1062357 on the mochitest-bc bustage and repushed this with a NS_ASSERTION: https://hg.mozilla.org/integration/mozilla-inbound/rev/10276ebe5711
Comment 35•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Attachment #8483248 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 37•10 years ago
|
||
Updated•10 years ago
|
status-b2g-v2.0:
--- → unaffected
status-b2g-v2.0M:
--- → unaffected
status-b2g-v2.1:
--- → fixed
status-b2g-v2.2:
--- → fixed
Comment 39•10 years ago
|
||
FWIW, I just got a telemetry regression alert about this:
> This alert was generated because the distribution of the histogram GHOST_WINDOWS
> has changed on the 2014-08-30. Please have a look at the following plot:
> http://vitillo.github.io/cerberus/dashboard/#2014-08-30GHOST_WINDOWS
Comment 40•10 years ago
|
||
Hi Nicholas, I'm not sure what the telemetry alert means - should there be a new bug filed? Or is it a suggestion that this may not be as fixed as one might hope? Those are some cool charts though. Wow.
Flags: needinfo?(n.nethercote)
Comment 41•10 years ago
|
||
> Hi Nicholas, I'm not sure what the telemetry alert means - should there be a
> new bug filed? Or is it a suggestion that this may not be as fixed as one
> might hope? Those are some cool charts though. Wow.
The telemetry alert arrived 8 or 9 days after the date of the regression. During that time the bug was fixed. So the alert is basically a late confirmation that there was a regression.
There hasn't yet been a subsequent telemetry alert telling us that this patch has fixed the problem. But the alerts seem to only happen occasionally, and the most recent data for GHOST_WINDOWS is still from 2014-08-30. Hopefully when the next one comes in it'll show the regression has fixed.
So, basically, everything looks reasonable for the moment. Sorry for the confusion.
Flags: needinfo?(n.nethercote)
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•