1048833 - [MTBF] Memory grows unexpectedly in system app

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect, P1)

Description

[Paul Yang [:pyang] (away)]

Reproduce by
Device:   Flame
Memory:   319MB
Gaia      9e5907995c9327f14cb5d182cee5ff16b1743ed4
Gecko     https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/3f7db58a354c
BuildID   20140803160201
Version   32.0
ro.build.version.incremental=109
ro.build.date=Mon Jun 16 16:51:29 CST 2014

From memory report, system app seems held over 1k event handlers and trigger LMK repeatedly.

Comment 1

[Paul Yang [:pyang] (away)]

URL for memory report: https://drive.google.com/file/d/0BzGnh6LcgHLQSnE3dmZ5UldiSTQ/edit?usp=sharing

Comment 2

[Cervantes Yu [:cyu] [:cervantes]]

Not sure if this is relevant, Ting-Yuan and I found that there are tons of "Function" entries in gc-edges.290.1407222245.log:

> cervantes@cyu-dt:/tmp/about-memory-1$ grep "G Function onInnerWindowDestroyed" gc-edges.290.1407222245.log |wc -l
39397
> cervantes@cyu-dt:/tmp/about-memory-1$ grep "G Function SpecialPowers" gc-edges.290.1407222245.log |wc -l
39411
> cervantes@cyu-dt:/tmp/about-memory-1$grep "G Function get" gc-edges.290.1407222245.log |wc - l
39425

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

It looks relevant.  There are 39361 inner-window-destroyed observers.

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

And the 'script' on all of the functions is chrome://specialpowers/content/specialpowers.js:28.

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

And if you search for '_pongHandlers' in the GC log you'll see there's ~39000 SpecialPowers objects.  This seems like a test harness issue.

Comment 6

[Paul Yang [:pyang] (away)]

ni? manili for special power usage in marionette

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Does marionette reuse the sandbox it creates to execute scripts in?

Comment 8

[Malini Das [:mdas] - Away, not checking bugmail]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #7)
> Does marionette reuse the sandbox it creates to execute scripts in?

By default, we don't reuse sandboxes. They get reused if you call execute script with 'newSandbox' parameter set to false.

When creating new sandboxes, we don't explicitly delete the old one, we just reassign the variable (https://mxr.mozilla.org/mozilla-central/source/testing/marionette/marionette-listener.js#482). This should be fixed, and I'll add a patch for this.

Comment 9

[Malini Das [:mdas] - Away, not checking bugmail]

(In reply to Paul Yang [: pyang] from comment #6)
> ni? manili for special power usage in marionette

What exactly is the question?

Comment 10

[Malini Das [:mdas] - Away, not checking bugmail]

(In reply to Malini Das [:mdas] from comment #8)
> (In reply to Kyle Huey [:khuey] ([e-mail]) from comment #7)
> > Does marionette reuse the sandbox it creates to execute scripts in?
> 
> By default, we don't reuse sandboxes. They get reused if you call execute
> script with 'newSandbox' parameter set to false.
> 
> When creating new sandboxes, we don't explicitly delete the old one, we just
> reassign the variable
> (https://mxr.mozilla.org/mozilla-central/source/testing/marionette/
> marionette-listener.js#482). This should be fixed, and I'll add a patch for
> this.

Actually, heh, I got confused and I'm not sure if there is a better way to do this. Do you know of a way to mark the content of a variable for garbage collection? I thought that by reassigning the variable, the object would mark itself for GC if it has no other variables referencing it.

Comment 11

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

The sandboxes are getting GCd, but the SpecialPowers stuff is not.  SpecialPowers has an implicit assumption that there's only going to be one per window.

Comment 12

[Malini Das [:mdas] - Away, not checking bugmail]

bumping to P1 for memory leaks leading to improper testing

Comment 13

[Malini Das [:mdas] - Away, not checking bugmail]

Marionette adds the specialpowers object to the window each time (https://mxr.mozilla.org/mozilla-central/source/testing/marionette/marionette-listener.js#405). I'll modify this so marionette will look for specialpowers first, and injects it if it is not found.

Comment 14

[Malini Das [:mdas] - Away, not checking bugmail]

Attachment: prevent unnecessary SpecialPowers loading

Before this patch, if you used SpecialPowers, then the SpecialPowers constructor would be called each time if you're not reusing a sandbox. I tested this patch locally on a browser and the SpecialPowers constructor doesn't get called each time you use it.

Mn desktop try is green:
https://tbpl.mozilla.org/?tree=Try&rev=db9123df5e53

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/83bc4242fd1c

Comment 16

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Not sure if we need blocking status to be able to land Marionette fixes but this does block our testing so here you go.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/a5594f074a65
https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/6c74f9535125

I was going to land it a=test-only anyway :)

Comment 18

[Malini Das [:mdas] - Away, not checking bugmail]

Can we mark as Fixed or are any other landings needed?

Comment 19

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

It hasn't landed on m-c yet which is why it's not resolved.

Comment 20

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

https://hg.mozilla.org/mozilla-central/rev/83bc4242fd1c

Comment 21

[Paul Yang [:pyang] (away)]

Thanks for the patch. It works really good.