Closed
Bug 899328
Opened 12 years ago
Closed 12 years ago
window manager is leaking apploaded event listeners causing long CC times
Categories
(Firefox OS Graveyard :: Gaia, defect, P1)
Tracking
(blocking-b2g:leo+, b2g18 fixed, b2g-v1.1hd fixed)
RESOLVED
FIXED
blocking-b2g | leo+ |
People
(Reporter: mccr8, Assigned: justin.lebar+bug)
References
Details
(Whiteboard: [MemShrink])
Attachments
(2 files)
After 3 days of running a test that opens an app every 10 seconds, we end up with 7000 apploaded event listeners registered on an iframe element. See bug 898990 for further details.
apploaded is defined here:
http://mxr.mozilla.org/gaia/source/apps/system/js/window_manager.js#800
These event listeners are being marked gray, so this is probably contributing to the long CC times in the test. It would be good to not leak those event listeners, but it is also a little odd that they aren't being optimized out of the CC graph.
Reporter | ||
Comment 1•12 years ago
|
||
These event listeners are all registered on a single iframe:
0x4c25c1c0 [FragmentOrElement (xhtml) iframe class='' app://system.gaiamobile.org/index.html]
--[[via hash] mListenerManager]--> 0x4a9025b0 [nsEventListenerManager]
--[mListeners[i] mListener]--> 0x4506bc08 [nsXPCWrappedJS (nsIDOMEventListener)]
--[mJSObj]--> 0x44a26e20 [JS Object (Function - apploaded)]
It looks like this this iframe is probably legitimately alive, so we're just failing to unregister the event listener when something dies, maybe?
The reflector for the iframe is being held alive by a path that looks kind of like this, so maybe this is a real iframe for the homescreen app:
--[WindowManager]--> 0x44a9df70 [Object <no private>]
--[retrieveHomescreen]--> 0x44adfa60 [Function retrieveHomescreen]
--[fun_callscope]--> 0x44a9dee0 [Call <no private>]
--[runningApps]--> 0x44ad8310 [Object <no private>]
--[app://homescreen.gaiamobile.org]--> 0x4c485b80 [Object <no private>]
--[iframe]--> 0x4c48f9a0 [HTMLIFrameElement 4b459cd0]
Comment 2•12 years ago
|
||
Well, just having thousands of extra objects in jsholders hashtable sure makes CC slower.
Reporter | ||
Comment 3•12 years ago
|
||
Yeah, and having to UnmarkGray 15000 things is probably going to be kind of slow, too...
Reporter | ||
Updated•12 years ago
|
Assignee | ||
Comment 4•12 years ago
|
||
To be clear, these event listeners are either 'mozbrowserloadend' or 'appopen', I think. We don't know which. The code is in window_manager.js.
Reporter | ||
Comment 5•12 years ago
|
||
Oh, okay, these listeners aren't being optimized out because this is an AllTraces log. In a real CC, they are hopefully being optimized out, but I'd guess we're still getting huge pauses (maybe even in the CC cleanup?) because there are just so many objects. That's just conjecture without a non-AllTraces CC log, however.
Assignee | ||
Comment 6•12 years ago
|
||
What's ironic about all this is that the only reason we have this event listener is to dispatch apploadtime, which is purely diagnostic.
I think we can rewrite this to use just one event listener.
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → justin.lebar+bug
Assignee | ||
Comment 7•12 years ago
|
||
Oh, and the timing is half-broken; it only works for cold-launches of apps, in v1-train. For warm launches, it just displays 0.
To enable app timing, dig through settings:
- device information
- more information
- developer
- show time to load
Assignee | ||
Comment 8•12 years ago
|
||
> Oh, and the timing is half-broken; it only works for cold-launches of apps, in v1-train.
But this is fixed in master. I guess we just didn't want to take the one-character fix on v1-train.
Assignee | ||
Comment 9•12 years ago
|
||
FTR:
diff --git a/apps/system/js/window_manager.js b/apps/system/js/window_manager.js
index 8166395..f4e9580 100644
--- a/apps/system/js/window_manager.js
+++ b/apps/system/js/window_manager.js
@@ -403,7 +403,7 @@ var WindowManager = (function() {
origin: displayedApp,
isHomescreen: (manifestURL === homescreenManifestURL)
});
- frame.dispatchEvent(evt);
+ iframe.dispatchEvent(evt);
}
// Executes when app closing transition finishes.
Reporter | ||
Updated•12 years ago
|
Whiteboard: [MemShrink]
Assignee | ||
Comment 10•12 years ago
|
||
Pointer to Github pull-request
Assignee | ||
Updated•12 years ago
|
Attachment #782867 -
Flags: review?(alive)
Assignee | ||
Comment 11•12 years ago
|
||
Pointer to Github pull-request
Assignee | ||
Updated•12 years ago
|
Attachment #782871 -
Attachment description: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/11238 → Pointer to Github pull request for v1-train.
Attachment #782871 -
Flags: review?(alive)
Updated•12 years ago
|
Attachment #782867 -
Flags: review?(alive) → review+
Updated•12 years ago
|
Attachment #782871 -
Flags: review?(alive) → review+
Assignee | ||
Comment 12•12 years ago
|
||
Merged to master: https://github.com/mozilla-b2g/gaia/commit/4b8a6d1d2aa3f5557b66c9863e397d2946e4e69b
and to v1-train (blocks a blocker): https://github.com/mozilla-b2g/gaia/commit/6c635e809baedb6b3393bd351b8ce95304782ac6
Assignee | ||
Comment 13•12 years ago
|
||
Thanks, Alive.
Assignee | ||
Updated•12 years ago
|
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 14•12 years ago
|
||
v1.1.0hd: 6c635e809baedb6b3393bd351b8ce95304782ac6
v1.1.0hd: dea92acb6be86623b9f66e196ef39bb895c4bd37
You need to log in
before you can comment on or make changes to this bug.
Description
•