Katelyn Gadd (:kael) Reporter
	Description • 11 years ago

Loading the URL attached to this bug seems to leak a ton of memory in the 'event-targets' category. I'm not sure why. This leak seems to have appeared once I started loading assets from TAR files, so I suspect it could be related to the use of XHR (more specifically, streaming text-mode XHR).

It doesn't leak in Chrome (the total heap size after starting according to Chrome's profiler is something like 47MB).

Snippet from about:memory :

526.83 MB (100.0%) -- explicit
├──345.54 MB (65.59%) -- window-objects
│  ├──336.90 MB (63.95%) -- top(http://hildr.luminance.org/bugs/3/Lumberjack/Lumberjack.html, id=8)
│  │  ├──303.65 MB (57.64%) -- active/window(http://hildr.luminance.org/bugs/3/Lumberjack/Lumberjack.html)
│  │  │  ├──154.25 MB (29.28%) -- js-compartment(http://hildr.luminance.org/bugs/3/Lumberjack/Lumberjack.html)
│  │  │  │  ├──118.24 MB (22.44%) -- objects-extra
│  │  │  │  │  ├──114.13 MB (21.66%) ── elements/non-asm.js
│  │  │  │  │  └────4.11 MB (00.78%) ── slots
│  │  │  │  ├───30.67 MB (05.82%) -- gc-heap
│  │  │  │  │   ├──25.49 MB (04.84%) -- objects
│  │  │  │  │   │  ├──13.35 MB (02.53%) ── ordinary
│  │  │  │  │   │  ├───6.29 MB (01.19%) ── dense-array
│  │  │  │  │   │  └───5.85 MB (01.11%) ── function
│  │  │  │  │   └───5.18 MB (00.98%) ++ (3 tiny)
│  │  │  │  └────5.34 MB (01.01%) ++ (4 tiny)
│  │  │  ├──149.17 MB (28.32%) -- dom
│  │  │  │  ├──113.68 MB (21.58%) ── event-targets [2]
│  │  │  │  ├───35.37 MB (06.71%) ── text-nodes
│  │  │  │  └────0.13 MB (00.02%) ++ (3 tiny)
│  │  │  └────0.23 MB (00.04%) ++ (2 tiny)
│  │  └───33.25 MB (06.31%) -- js-zone(0xd426800)
│  │      ├──29.70 MB (05.64%) -- gc-heap
│  │      │  ├──23.89 MB (04.54%) ── unused-gc-things
│  │      │  └───5.81 MB (01.10%) ++ (4 tiny)
│  │      └───3.54 MB (00.67%) ++ (4 tiny)
│  └────8.64 MB (01.64%) ++ (6 tiny)
├──100.41 MB (19.06%) ── heap-unclassified
├───41.71 MB (07.92%) -- js-non-window
│   ├──19.33 MB (03.67%) -- zones
│   │  ├───9.96 MB (01.89%) ++ zone(0x4045800)
│   │  ├───8.71 MB (01.65%) ++ zone(0xa72800)
│   │  └───0.67 MB (00.13%) ++ (6 tiny)
│   ├──17.33 MB (03.29%) -- runtime
│   │  ├───6.60 MB (01.25%) ── script-sources
│   │  ├───6.07 MB (01.15%) ── script-data
│   │  └───4.67 MB (00.89%) ++ (10 tiny)
│   └───5.04 MB (00.96%) ++ gc-heap
├───11.45 MB (02.17%) ++ (16 tiny)
├────9.29 MB (01.76%) -- dom
│    ├──9.24 MB (01.75%) -- memory-file-data
│    │  ├──9.14 MB (01.74%) ++ large
│    │  └──0.10 MB (00.02%) ── small
│    └──0.05 MB (00.01%) ── event-listener-managers-hash
├────7.44 MB (01.41%) -- images
│    ├──7.23 MB (01.37%) -- content
│    │  ├──7.08 MB (01.34%) -- used
│    │  │  ├──6.60 MB (01.25%) ── raw
│    │  │  └──0.48 MB (00.09%) ++ (2 tiny)
│    │  └──0.15 MB (00.03%) ++ unused
│    └──0.21 MB (00.04%) ++ chrome
├────5.66 MB (01.07%) ++ workers/workers()
└────5.33 MB (01.01%) ++ storage

Tested in Nightly 25.0a1 as of today (6/25). Leak does *not* reproduce in Firefox 21 (release channel); looks like maybe it reproduces in Firefox 22 (release channel) but the memory usage is less.

	Katelyn Gadd (:kael) Reporter
	Comment 1 • 11 years ago

Disabling sound in the test case since it hits another open bug.

	(no longer active)
	Comment 2 • 11 years ago

XHRs are event targets, so your theory is plausible.

	Kyle Huey (Exited; not receiving bugmail, old account, do not use)
	Comment 3 • 11 years ago

Yeah, among other things it measures XHRs, and their responseBody/text.

	(no longer active)
	Comment 4 • 11 years ago

Alice, do you think you have some time to find a regression range for this bug?  Thanks a lot!

	Alice0775 White
	Comment 5 • 11 years ago

STR?

I cannot reproduce the problem.
"event-target" disappears immediately after I close the page in Nightly25.0a1.

	(no longer active)
	Comment 6 • 11 years ago

(In reply to Alice0775 White from comment #5)
> STR?

I believe that Kevin is using the link in the URL field.  Kevin, do you mind providing steps to reproduce?  Thanks!

	Katelyn Gadd (:kael) Reporter
	Comment 7 • 11 years ago

Load the URL and then open about:memory while the page is loaded.

Sorry if I was unclear; this is an application leak (the event target memory should not persist after the app is loaded), not a browser framework leak.

	Katelyn Gadd (:kael) Reporter
	Comment 8 • 11 years ago

Ran mozregression against this and it looks like maybe it's just that the memory reporters added a category for event targets. The total memory usage for the app once loaded doesn't appear to have gone up very much; if anything it's the same.


Last good nightly: 2013-01-20
First bad nightly: 2013-01-21

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=01a8559f5560&tochange=01c964705be7


However, I do see the event-target memory used vary - between 30 and 100MB - so maybe there is still a bug here that is causing some amount of event target memory to leak that varies based on how many XHR events are fired. Not sure how to debug that (is there a way to trace live references in the spidermonkey heap, atm?)

	Katelyn Gadd (:kael) Reporter
	Comment 9 • 11 years ago

Ah, yeah, there it is in the pushlog:
http://hg.mozilla.org/mozilla-central/rev/de2ab911692d

	Katelyn Gadd (:kael) Reporter
	Comment 10 • 11 years ago

OK, this may be intermittent or entirely based on when the XHR events fire/how many they are, but running mozregression I did see the amount of event target memory increase in recent builds:

Last good nightly: 2013-06-11
First bad nightly: 2013-06-12

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9115d8b717e1&tochange=0414d6d0f60d

(That window is probably too narrow; not sure what to do about it)

In older builds (say from 3 months ago), the event target memory reported is usually around 30-40mb; in these newer builds the reported memory is 80-120mb.

It is possible this is entirely an application level bug and browser behavior just changed, but this application doesn't leak any XHR memory (that I'm aware of) in Chrome according to their heap profiler. It used to leak some, but I fixed all the leaks.

	Alice0775 White
	Comment 11 • 11 years ago

After landing Bug 678037, memory of "event-target" is not released.

Steps To Reproduce:
1. Open URL
2. Wait to disappear "Downloading progress bar"
3. Open about:memory in new tab while processing "Loading progress bar" and Click "Measure"
   --- observe memory of "event-target"
4. Wait for 20 seconds, and Open about:memory in new tab and Click "Measure"
   --- observe memory of "event-target" again

Actual Results:
   At step 3, about 115MB
   At step 4, about 70MB, memory of "event-target" is not released

Expected Results:
   At step 3, about 115MB
   At step 4, no "event-target"


Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/5ddb1bf96261
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20130614 Firefox/24.0 ID:20130614100407
Bad:
http://hg.mozilla.org/mozilla-central/rev/05d9196b27a1
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20130614 Firefox/24.0 ID:20130614184124
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5ddb1bf96261&tochange=05d9196b27a1


Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/18c1fd169792
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20130614 Firefox/24.0 ID:20130614031707
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/ce43d28276e4
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20130614 Firefox/24.0 ID:20130614045911
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=18c1fd169792&tochange=ce43d28276e4


Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/ccd298a9db28
Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20130614 Firefox/24.0 ID:20130614044305
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/ce43d28276e4
Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20130614 Firefox/24.0 ID:20130614045911
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ccd298a9db28&tochange=ce43d28276e4

Regressed by:
ce43d28276e4	Brian Hackett — Bug 678037 - Enable lazy JS parsing and fix various bugs, r=waldo,evilpie,nobody.

	Brian Hackett [Laid off!] Assignee
	Comment 12 • 11 years ago

I'm trying to reproduce this but eventually the 'downloading' page just stalls and I get messages like "The assert 'Shaders/BloomCombine.xnb' could not be loaded ..."

	Katelyn Gadd (:kael) Reporter
	Comment 13 • 11 years ago

What build of firefox are you using to test? What platform?

	Katelyn Gadd (:kael) Reporter
	Comment 14 • 11 years ago

P.S. I just loaded the test URL in Nightly and it started fine and I see some event-targets memory usage (a bit lower, but as noted it varies some - perhaps based on timing/caching):

├───69.81 MB (17.55%) ── event-targets [2]

I'm able to load it successfully in nightlies from 07-01, 07-02, and 07-10. I see at least 70mb of event targets memory in each.

	Brian Hackett [Laid off!] Assignee
	Comment 15 • 11 years ago

I'm using my own debug build off of tip.  Can you put up a zip or something with the entire site so I can try it locally?

	Katelyn Gadd (:kael) Reporter
	Comment 16 • 11 years ago

https://dl.dropboxusercontent.com/u/1643240/887075.zip

	Brian Hackett [Laid off!] Assignee
	Comment 17 • 11 years ago

Thanks, I was able to reproduce the problem with that zip.

I think this patch fixes the problem.  At least, when I get to the game screen (after the downloading and loading screens) and minimize memory there is no event-targets memory.  What I think is going on is that when lazily parsing a script the function associated with that script is whichever function triggered the lazy parse.  Compared to the canonical function for the script, which this used to be and really should be, this function can entrain extra data in its data properties or, more likely, environment / scope chain.  This patch stores the canonical function on the lazy script (replacing a padding field on 32 bit platforms so not increasing memory usage there) so that the things held live by a script are the same whether lazy parsing is in use or not.

	Brian Hackett [Laid off!] Assignee
	Comment 18 • 11 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/1ef79950f0ab

	Andrew McCreight [:mccr8]
	Comment 19 • 11 years ago

Thanks for reporting this, Kevin, this turns out to be a big problem on b2g trunk.

	Ed Morley [:emorley]
	Comment 20 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/1ef79950f0ab

	Andrew McCreight [:mccr8]
	Comment 21 • 11 years ago

It looks like this still needs to be landed on Aurora.

	Brian Hackett [Laid off!] Assignee
	Comment 22 • 11 years ago

Comment on attachment 774942 [details] [diff] [review]
patch

Oops, yes.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 678037
User impact if declined: Unnecessary GC edges which can hold onto things unnecessarily
Testing completed (on m-c, etc.): on m-c for the last week
Risk to taking this patch (and alternatives if risky): none

	Andrew McCreight [:mccr8]
	Comment 23 • 11 years ago

To elaborate a bit on the user impact, jlebar and I observed this causing huge leaks on B2G, and this bug is about a big leak it caused in completely different code, so presumably this could cause random huge leak problems everywhere.

	Ryan VanderMeulen [:RyanVM]
	Comment 24 • 11 years ago

Needs a branch-specific patch for uplift.

	Brian Hackett [Laid off!] Assignee
	Comment 25 • 11 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/39294d14e111

	Ioana (away)
	Comment 26 • 11 years ago

Verified as fixed on Firefox 24 RC - 20130910201120.

The memory consumption is still as high, but event-targets is released after the download ends.

	Ioana (away)
	Comment 27 • 11 years ago

Also verified on the Firefox 25 beta 1 build (20130917123208). Same results as above.