Build Identifier: http://hg.mozilla.org/mozilla-central/rev/ce20e9b47e9c Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120225 Firefox/13.0a1 ID:20120225031723 When I test Bug 730698 attachment 600788 [details], browser crashes bp-20846c43-1e86-459b-a638-e25832120226 Reproducible: Always Steps to Reproduce: 1. Start Firefox with new profile 2. Run firefoxcrashtest.html of attachment 600788 [details] Actual Results: Browser crashes Expected Results: No crash Regression window(m-c) Browser does not crash for about 1 min. And I can barely operate it: http://hg.mozilla.org/mozilla-central/rev/f410bdf30132 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0a1) Gecko/20120130 Firefox/12.0a1 ID:20120130122131 Browser crashes within 30sec: http://hg.mozilla.org/mozilla-central/rev/89bb79343f73 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0a1) Gecko/20120130 Firefox/12.0a1 ID:20120130125731 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f410bdf30132&tochange=89bb79343f73

This sounds like a CC regression...

My initial guess is that we end up calling CC fewer times, so we don't release memory fast enough.

Unfortunately I can't reproduce this on linux.

I'll post a patch to tryserver. Hopefully someone can test it.

This brings back similar behavior what we have in <FF12. Yes, there can be some more CCs, but fortunately they are a lot faster. http://mxr.mozilla.org/mozilla-beta/source/dom/base/nsJSEnvironment.cpp#3430 http://mxr.mozilla.org/mozilla-beta/source/dom/base/nsJSEnvironment.cpp#3339 https://tbpl.mozilla.org/?tree=Try&rev=26f8245eb5b0 Please test the tryserver build Look for 26f8245eb5b0 in http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/

Comment on attachment 600877 [details] [diff] [review] patch Review of attachment 600877 [details] [diff] [review]: ----------------------------------------------------------------- Makes sense. We really have no idea how much garbage a GC could have released, so we should err on the side of caution. ::: dom/base/nsJSEnvironment.cpp @@ +180,5 @@ > static PRUint32 sForgetSkippableBeforeCC = 0; > static PRUint32 sPreviousSuspectedCount = 0; > > static bool sCleanupSinceLastGC = true; > +static bool sNeedsFullCC = false; Do you need to set this to false in that init function, too?

(In reply to Andrew McCreight [:mccr8] from comment #6) > > +static bool sNeedsFullCC = false; > > Do you need to set this to false in that init function, too? I could do that, just to be consistent. (Someone should cleanup nsJSEnvironment.cpp)

Comment on attachment 600897 [details] [diff] [review] patch [Approval Request Comment] Regression caused by (bug #): bug 721543 User impact if declined: possible OOM in rare cases Testing completed (on m-c, etc.): just landed to m-c Risk to taking this patch (and alternatives if risky): bringing back old behavior, should be low risk. String changes made by this patch: N/A

Could someone please test http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/opettay@mozilla.com-26f8245eb5b0/

I wonder if this will help with the intermittent hang I was getting in bug 727965.

(In reply to Olli Pettay [:smaug] from comment #11) > Could someone please test > http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/opettay@mozilla. > com-26f8245eb5b0/ The try build crashes after 15-30sec :(

Boo. Then it is about something else. We should still use the patch. Could you check if GC and/or CC runs while running the test. Set javascript.options.mem.log to true and check error console Messages.

And latest m-c tinderbox-builds also crashes after 15-30sec. bp-2176ffdf-c6e3-4216-9f32-625232120227 http://hg.mozilla.org/mozilla-central/rev/8ea5c983743f Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120227 Firefox/13.0a1 ID:20120227065949

Sorry, Accidentally removed flags

(In reply to Olli Pettay [:smaug] from comment #14) > Could you check if GC and/or CC runs while running the test. > Set javascript.options.mem.log to true and check error console Messages. It is difficult to capture the log because the browser crashes and error console closes at same time. GC(T+12.8) TotalTime: 3.4ms, Type: compartment, MMU(20ms): 83%, MMU(50ms): 93%, Reason: TOO_MUCH_MALLOC, +chunks: 0, -chunks: 0 mark: 1.5, mark-roots: 1.1, mark-other: 0.0, sweep: 1.6, sweep-obj: 0.5, sweep-string: 0.0, sweep-script: 0.0, sweep-shape: 0.0, discard-code: 0.0, discard-analysis: 0.1, xpconnect: 0.6, deallocate: 0.0 GC(T+13.6) TotalTime: 2.7ms, Type: compartment, MMU(20ms): 86%, MMU(50ms): 94%, Reason: TOO_MUCH_MALLOC, +chunks: 0, -chunks: 0 mark: 1.2, mark-roots: 0.9, mark-other: 0.0, sweep: 1.3, sweep-obj: 0.4, sweep-string: 0.0, sweep-script: 0.0, sweep-shape: 0.0, discard-code: 0.0, discard-analysis: 0.1, xpconnect: 0.4, deallocate: 0.0 GC(T+14.5) TotalTime: 2.9ms, Type: compartment, MMU(20ms): 85%, MMU(50ms): 94%, Reason: TOO_MUCH_MALLOC, +chunks: 0, -chunks: 0 mark: 1.2, mark-roots: 0.9, mark-other: 0.0, sweep: 1.5, sweep-obj: 0.5, sweep-string: 0.0, sweep-script: 0.0, sweep-shape: 0.0, discard-code: 0.0, discard-analysis: 0.1, xpconnect: 0.5, deallocate: 0.0

Comment #17 is the Nightly nightly build. http://hg.mozilla.org/mozilla-central/rev/d1b2fd680235 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120227 Firefox/13.0a1 ID:20120227031120

Oh, only compartment GC. That doesn't trigger CC.

Have we changed when full GC is triggered?

Comment on attachment 600897 [details] [diff] [review] patch [Triage Comment] Not clear this is absolutely necessary, but restores old behavior so approving for Aurora 12.

Is it fixed in the trunk?

This depends on bug 730853. The patch fixes one issue.

With this patch (well, and my patch in bug 728460), I'm seeing about 80% of CCs being triggered by this force CC. It kind of makes the CC hostage to the GC if we want to try to call the CC less. Kind of a shame, but I think this patch is probably the way to go, because otherwise you'll hit situations like this that are problematic.

Olli: would it make sense to trigger a CC every 5 or 10 compartmental GCs?

This probably belongs in another bug, but another approach to getting the GC to trigger a CC would be to dump all wrapped natives (etc.) into the purple buffer at the conclusion of a GC. Then we could filter out everything held by a marked object using the normal methods. That would probably work for this particular case, where it is allocating a huge number of cross-compartment cycles.

Bug 730853 didn't land in 12.0 so it's not fully fixed.

(In reply to Scoobidiver from comment #29) > Bug 730853 didn't land in 12.0 so it's not fully fixed. Given the original risk evaluation that this would only occur in rare cases, untracking for 12 since we wouldn't take any more forward change for this.

I think this is finished, correct?