I see a regression reported on dev.tree-management: https://groups.google.com/forum/#!topic/mozilla.dev.tree-management/XHDWPVFKjGw this is seen in the graphs view: http://graphs.mozilla.org/graph.html#tests=[[257,131,37],[257,131,25],[257,131,35],[257,131,33]]&sel=1382976298751,1385568298751&displayrange=30&datatype=running This takes place during the tp5o test run (we load 49 different locally cached websites) this is a noticeable regression on windows xp and windows 7, but an improvement on ubuntu 12.04 (32 and 64 bit). Looking at the graphs and retriggering a few runs, the changeset that stands out is from bug 942984: https://hg.mozilla.org/integration/mozilla-inbound/rev/c5714ab2828b

:bhacket- any thoughts if this is related to your patch?

The only relevant change this patch does is specify a stack size for JS worker threads. The size it specifies is 512K. Previously it used the default system thread stack size, which from bug 942984 comment 4 and http://msdn.microsoft.com/en-us/library/windows/desktop/ms686774%28v=vs.85%29.aspx is 1MB on Windows, so if anything this bug should have improved Windows memory usage.

Well, this sure looks like a 2-3% regression to me. Joel, do you want to try experimentally backing it out?

If we assume that whatever change caused Ubuntu (green and orange) to jump down also caused the regression on Windows (red and blue), then it's one of these: 157706 076bf878c59a 2013-11-27 03:40 +0200 Olli Bug 942432 - Remove nsIJSEventListener::mContext, r=bz 157707 db836ecd7746 2013-11-27 03:42 +0200 Olli Bug 943613 - Notify JS implemented Event Target when an event listener is added / removed, r=bz 157708 551b2064b705 2013-11-26 18:06 -0800 shu Bug 937763 - Don't emit MIR marked as emittedAtUses immediately when redefining. (r=jandem) 157709 62e94f70b2cd 2013-11-26 19:13 -0700 bhackett1024 Bug 939088 - Add a cache for fetching the names associated with ALIASEDVAR operations, r=luke. 157710 c5714ab2828b 2013-11-26 19:18 -0700 bhackett1024 Bug 942984 - Set native stack limit for JS worker threads, r=billm. 157711 3964ce61dacf 2013-11-26 21:29 -0500 ehsan Bug 943554 - Extend the checks added in bug 941854 to all unified files; r=gps 157712 b66f95073899 2013-11-26 22:14 -0500 ehsan Backed out changeset db836ecd7746 (bug 943613) for build bustage 157713 9849749f3623 2013-11-26 22:14 -0500 ehsan Backed out changeset 076bf878c59a (bug 942432) for build bustage but maybe jmaher has even more specific information than what shows up on the graph.

I have no other specific info, let me do a try run and back out some of these patches

I did a try run with the patch from bug 942984 backed out: https://tbpl.mozilla.org/?tree=Try&rev=bfea8446c907 our private bytes did go down. Is this something we want to live with? The change is minimal, but it is showing a memory increase. :bhackett- any more thoughts on this?

This patch reverts things to use the default stack size on Windows. Joel, can you see if this patch also improves the private bytes? If so, since this actually increases the stack space available to these threads, this would indicate a bug in NSPR and/or Windows.

I pushed that patch to try: https://tbpl.mozilla.org/?tree=Try&rev=d93eac6e2e2f the numbers are falling in line with the original numbers, it appears to fix the problem! Is this something we can land? Need me to try anything else out?

Comment on attachment 8343764 [details] [diff] [review] use default stack size on windows Review of attachment 8343764 [details] [diff] [review]: ----------------------------------------------------------------- Perhaps Windows doesn't commit the memory if you pass 0, but it does if you pass something else. Anyway, it's not worth investigating.

However, once this lands, it would be great if someone from QA could verify that bug 942984 is still fixed on Windows. There's a small possibility that this patch will regress that fix.

Comment on attachment 8343764 [details] [diff] [review] use default stack size on windows [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 942984 User impact if declined: maybe increased memory usage on windows Testing completed (on m-c, etc.): on m-i Risk to taking this patch (and alternatives if risky): low, per msdn documentation this should not rebreak bug 942984.

(In reply to Bill McCloskey (:billm) from comment #10) > However, once this lands, it would be great if someone from QA could verify > that bug 942984 is still fixed on Windows. There's a small possibility that > this patch will regress that fix. I'll flag that bug for testing again and follow-up there.