517665 - XPCJSStackFrame::CreateStack cause permanent memory grow ~1Mb per second

Status: RESOLVED FIXED

(Core :: XPConnect, defect, P2)

Description

[Oleg Romashin (:romaxa)]

Attachment: mybrowser

Download:
http://ftp.mozilla.org/pub/mozilla.org/xulrunner/nightly/latest-mozilla-1.9.2/xulrunner-1.9.2a2pre.en-US.linux-i686.tar.bz2

http://benjamin.smedbergs.us/xulrunner/mybrowser-0.2.2.xulapp - or take from attachment

./run-mozilla.sh ./xulrunner-bin /tmp/mybrowser/application.ini 

In parallel run (sp-memusage package from maemo)
mem-monitor-smaps -c -p xulrunner-bin

Open facebook.com page (startup page, no credentials available)
Reload that page 2-3 times (probably it is not required)

Check mem-monitor-smaps output

EXP: Memory is not growing
ACT:
for ./xulrunner-bin[21230] according to SMAPS.
(without swap as SMAPS doesn't report swap correctly)

		system	process		private	/------ dirty ---------\
time:		avail:	size:	rss:	clean:	shared:	private: change:
14:28:34	2810064	115764	38036	13320	0	18492	   +0 kB
14:28:38	2831840	116788	39060	13320	0	19516	+1024 kB
14:28:40	2844172	118836	37220	13324	0	17672	-1844 kB
14:28:43	2868524	118836	37996	13324	0	18448	 +776 kB
14:28:45	2812312	118836	38536	13324	0	18988	 +540 kB
14:28:47	2841360	118836	39256	13324	0	19708	 +720 kB
14:28:49	2842816	118836	40004	13324	0	20456	 +748 kB
14:28:51	2816892	118836	40460	13324	0	20912	 +456 kB
14:28:53	2819824	118836	41000	13324	0	21452	 +540 kB
14:28:55	2770896	118836	41112	13324	0	21564	 +112 kB
14:29:00	2857716	119860	42136	13324	0	22588	+1024 kB
14:29:02	2823132	120884	43160	13324	0	23612	+1024 kB
14:29:06	2839360	121908	44184	13324	0	24636	+1024 kB
14:29:08	2782144	122932	45208	13324	0	25660	+1024 kB
14:29:10	2857352	123956	46232	13324	0	26684	+1024 kB
14:29:14	2819664	124980	47256	13324	0	27708	+1024 kB
14:29:16	2836336	126004	48280	13324	0	28732	+1024 kB
14:29:20	2792724	127028	49304	13324	0	29756	+1024 kB
14:29:23	2790648	128052	50328	13324	0	30780	+1024 kB
14:29:25	2876252	129076	51352	13324	0	31804	+1024 kB
14:29:27	2831828	130100	52376	13324	0	32828	+1024 kB
14:29:31	2819000	131124	53400	13324	0	33852	+1024 kB
....
14:30:37	2756368	155700	77988	13308	0	58428	+1024 kB
....

Comment 1

[Oleg Romashin (:romaxa)]

Attachment: sp-memusage debian package

Comment 2

[Oleg Romashin (:romaxa)]

Attachment: Memory allocation backtrace of idle facebook login page

I found that facebook page executing script
http://static.ak.fbcdn.net/rsrc.php/z4QA3/hash/9y5lydl4.js:139
And it always throw exception...
Memory grow, but never free.

Comment 3

[Oleg Romashin (:romaxa)]

Attachment: Modified with "lowmem" button - heapminimize call

Also if I call memory-pressure/heapminimize, then memory returns back to the system (nsJSContext:CC() executed).
Or page reload
5:04:44	2844052	172104	86160	13276	0	66556	+1024 kB
15:04:48	2842636	173128	87184	13276	0	67580	+1024 kB
15:05:11	2835808	178256	92372	13276	0	72768	+5188 kB
15:05:13	2887204	174176	40480	13276	0	20876	-51892 kB
15:05:15	2887372	174176	40644	13276	0	21040	 +164 kB
15:05:17	2887440	174176	41228	13276	0	21624	 +584 kB
15:05:19	2886684	174176	41744	13276	0	22140	 +516 kB
15:05:21	2886716	174176	42356	13276	0	22752	 +612 kB

Comment 4

[Peter Van der Beken [:peterv]]

One problem might be that nsXPCException isn't cycle collected, so we don't try to CC even though a number of them got created (but even then, we don't take into account how big the objects are).

Comment 5

[Oleg Romashin (:romaxa)]

Attachment: Can we call CC sometime if use inactive?

Comment 6

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

Comment on attachment 401635 [details] [diff] [review]
Can we call CC sometime if use inactive?

I thought I reviewed this already.
I don't think we want to call CC too often. Better to fix nsXPCException handling.

Comment 7

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

(In reply to comment #6)
> Better to fix nsXPCException handling.
Or whatever causes this bug.

Romaxa, I'm having hard time trying to reproduce this.
Any help appreciated.

(Though, if the problem is something that only sp-memusage debian package shows,
I have the problem that I don't have debian nearby.)

Comment 8

[Oleg Romashin (:romaxa)]

> (Though, if the problem is something that only sp-memusage debian package
> shows,
> I have the problem that I don't have debian nearby.)

What do you have ? is it linux? if it is linux, then you can just unpack debian package and use library and binary from there... otherwise (Win/MacOS/Solaris) you better know some memory measurement (per process) tool for your system.

Comment 9

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

Romaxa, with this bug report, do you mean the loop is endless and you
constantly leak something, until a new page is loaded so that CC is triggered?

(I didn't use sp-memusage since I don't have any unpack deb tool here, and couldn't find such immediately. But other tools should work well enough.)

Comment 10

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

Romaxa, any help here would be great.
What kind of leak do you see? Is it a constant 1Mb per second? or what?
Hard to reproduce.

Comment 11

[Oleg Romashin (:romaxa)]

Checked again by using xulrunner:
http://hg.mozilla.org/mozilla-central/rev/1904598c4caf

extract mybrowser application from attachments:
./run-mozilla.sh ./xulrunner-bin  /tmp/mybrowser/application.ini

open facebook.com
run in other terminal:
while true; do cat /proc/30946/status  | grep Vm; sleep 1; done

reload it facebook page, keep mouse focus in browser facebook window (window is focused).

Check for VmRSS: state in other terminal.

You should see that it is growing.

Comment 12

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

(In reply to comment #11)
> Check for VmRSS: state in other terminal.
> 
> You should see that it is growing.
In my earlier testing I did see it growing a bit, but then it dropped down.
I'll test this again.

Comment 13

[Oleg Romashin (:romaxa)]

When test is started, don't do anything, don't touch mouse, don't change focus, no any actions which may cause user-active/inactive switch.

Comment 14

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

Attachment: for debugging

I wonder how to really make this work nicely.
Exceptions are "thread safe", so I don't think we can cycle collect all of them.
But would it be bad to have main thread only exceptions, which would be
CC-able. That might break something. And is ugly too.

Peterv, any suggestions?

Comment 15

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

Attachment: Cycle collectable exception container for main thread

Just an idea. Need to think about cross thread exception handling.

Comment 16

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

Attachment: testcase

Run in FF. Don't move mouse over the window.

Comment 17

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

So we create lots of XPCWrappedNative objects, but there is nothing
which would trigger cycle collector which could then clean up
unnecessary wrappers.

Comment 18

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

This is an old regression. 1.8.x cleans up the objects when GC runs.
1.9.x doesn't.

Comment 19

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

Or not quite that way. We don't seem to trigger gc often enough.
A patch coming.

Comment 20

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

Attachment: Trigger GC occasionally

This is pretty safe, but ugly hack to trigger gc occasionally.

Comment 21

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

Attachment: Count wrappednatives and call CC if there are many new ones

This works pretty well with the testcase and with mybrowser+facebook.
And pages like gmail or gmaps don't trigger more cycle collections than currently.

I'm not quite sure if sXPCWrappedNativeCount should count only
wrappers created in/for main thread, but seems like this approach is simpler 
and NS_MAX_NEW_WRAPPED_NATIVES is pretty high so it doesn't get triggered too
easily.

Note, this is 1.9.2+ so I don't want to do too big changes.

Comment 22

[Peter Van der Beken [:peterv]]

Since we're looking at hacks anyway and using the number of XPCWrappedNatives created to trigger GCs, can you try calling updateMallocCounter with the size of the XPCWrappedNative in XPCWrappedNative::FinishInit?

I'm not too fond of adding a counter for XPCWrappedNatives (btw, if we do we can probably use the cheaper non-atomic increment/decrement).

Comment 23

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

Attachment: updateMallocCounter

Instead of increasing memory usage 150MB, this increases only 100MB before GC.

Comment 24

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

Attachment: updateMallocCounter * 2

This gives a bit more reasonable behavior.
Pure hack.

Comment 25

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

Comment on attachment 406928 [details] [diff] [review]
updateMallocCounter * 2

Argh, this contains non-sense libffi.info part

Comment 26

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

Attachment: updateMallocCounter * 2

Comment 27

[Peter Van der Beken [:peterv]]

Comment on attachment 406933 [details] [diff] [review]
updateMallocCounter * 2

I don't like it, but I don't really know a fix I would like better.

Comment 28

[Peter Van der Beken [:peterv]]

Blake, Igor, hope you guys are ok with this too.

Comment 29

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 406933 [details] [diff] [review]
updateMallocCounter * 2

>diff --git a/js/src/xpconnect/src/xpcwrappednative.cpp b/js/src/xpconnect/src/xpcwrappednative.cpp
>+    // A hack for bug 517665, increase the probability for GC.
>+    JSContext* cx = ccx.GetJSContext();
>+    if (cx) {
>+        cx->updateMallocCounter(2 * sizeof(XPCWrappedNative));
>+    }

We cannot have a null cx here (the ccx would be invalid, which should have been caught earlier). I'm about as happy with the patch as everybody else, but given how much science has gone into GC until now (tongue firmly in cheek), this doesn't seem to be any worse than anything else we've done.

Comment 30

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

Attachment: no null check

Comment 31

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

http://hg.mozilla.org/mozilla-central/rev/554d3a8cc6ff

Comment 32

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Might this have caused a 0.50% DHTML increase on Windows Vista and 0.42% on Windows XP?

http://groups.google.com/group/mozilla.dev.tree-management/browse_thread/thread/2f320077b44e9660#
http://groups.google.com/group/mozilla.dev.tree-management/browse_thread/thread/d76713dcfc9ad993#

Comment 33

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

Not likely, but possible.

Comment 34

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

I yet decided to try this on 1.9.2.
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/3a99a35cf4c5

Comment 35

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

At least on 1.9.2 I can see any tdhtml changes on Windows.