899328 - window manager is leaking apploaded event listeners causing long CC times

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect, P1)

Description

[Andrew McCreight [:mccr8]]

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.

Comment 1

[Andrew McCreight [:mccr8]]

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

[Olli Pettay [:smaug][[e-mail]]]

Well, just having thousands of extra objects in jsholders hashtable sure makes CC slower.

Comment 3

[Andrew McCreight [:mccr8]]

Yeah, and having to UnmarkGray 15000 things is probably going to be kind of slow, too...

Comment 4

[Justin Lebar (not reading bugmail)]

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.

Comment 5

[Andrew McCreight [:mccr8]]

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.

Comment 6

[Justin Lebar (not reading bugmail)]

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.

Comment 7

[Justin Lebar (not reading bugmail)]

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

Comment 8

[Justin Lebar (not reading bugmail)]

> 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.

Comment 9

[Justin Lebar (not reading bugmail)]

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.

Comment 10

[Justin Lebar (not reading bugmail)]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/11236

Pointer to Github pull-request

Comment 11

[Justin Lebar (not reading bugmail)]

Attachment: Pointer to Github pull request for v1-train.

Pointer to Github pull-request

Comment 12

[Justin Lebar (not reading bugmail)]

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

Comment 13

[Justin Lebar (not reading bugmail)]

Thanks, Alive.

Comment 14

[Tim Guan-tin Chien [:timdream] (please needinfo)]

v1.1.0hd: 6c635e809baedb6b3393bd351b8ce95304782ac6
v1.1.0hd: dea92acb6be86623b9f66e196ef39bb895c4bd37