Closed
Bug 624867
Opened 13 years ago
Closed 2 years ago
Pauses in Flight of the Navigator due to MaybeGC() being called on every 20th script execution
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
INCOMPLETE
Tracking | Status | |
---|---|---|
blocking2.0 | --- | - |
People
(Reporter: pcwalton, Unassigned)
References
Details
(Whiteboard: [softblocker][not-ready])
Attachments
(2 files)
1.78 KB,
patch
|
jst
:
review+
joe
:
approval2.0-
|
Details | Diff | Splinter Review |
2.30 KB,
patch
|
Details | Diff | Splinter Review |
MaybeGC() gets called on every 20th script execution, causing jank in the Flight of the Navigator demo. I've created a patch to remove this call.
Reporter | ||
Comment 1•13 years ago
|
||
Proposed patch attached. Baking on try server.
Updated•13 years ago
|
Attachment #502945 -
Flags: review?(jst) → review+
Comment 2•13 years ago
|
||
After this, do we only do full gc if there is cycle collection with gc? (Note, it is possibly to run scripts without causing any cycle collections) Is it possible to have cross compartment wrappers or something which need to be collected occasionally using something like MaybeGC()?
Reporter | ||
Comment 3•13 years ago
|
||
(In reply to comment #2) > After this, do we only do full gc if there is cycle collection with gc? > (Note, it is possibly to run scripts without causing any cycle collections) MaybeGC() can call either, depending on some logic in the JSAPI: http://mxr.mozilla.org/mozilla-central/source/js/src/jsgc.cpp#1825 > Is it possible to have cross compartment wrappers or something which need to be > collected occasionally using something like MaybeGC()? Yeah, although the JS engine will call it automatically if it needs to I believe (although it's slower).
Reporter | ||
Comment 4•13 years ago
|
||
No notable regressions on tp4. Going to send to tracemonkey.
Reporter | ||
Updated•13 years ago
|
Attachment #502945 -
Flags: approval2.0?
Updated•13 years ago
|
blocking2.0: --- → betaN+
Reporter | ||
Updated•13 years ago
|
Whiteboard: fixed-in-tracemonkey
Updated•13 years ago
|
Attachment #502945 -
Flags: approval2.0? → approval2.0+
Comment 5•13 years ago
|
||
This landed a while ago: http://hg.mozilla.org/tracemonkey/rev/d1adb7b245bd http://hg.mozilla.org/mozilla-central/rev/d1adb7b245bd
Comment 6•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/d1adb7b245bd
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 7•13 years ago
|
||
Backed out in http://hg.mozilla.org/mozilla-central/rev/f28ee131df7a Was causing SunSpider regressions.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Updated•13 years ago
|
Whiteboard: fixed-in-tracemonkey
Version: unspecified → Trunk
Comment 8•13 years ago
|
||
So, could this cause the regression because these MaybeGC calls happen usually when we aren't running javascript, but just right after that, for example right after the function which was run because of some timeout. Now, if we take explicit MaybeGC calls out, GC may run more often while we're running the tests. I wonder if we could change (mNumEvaluations > 20). That 20 has been there forever. Also, could we perhaps explicitly do "compartment only gc" more often and then call MaybeGC every now and then? /me still has no idea how to explicitly call only compartment gc.
Comment 9•13 years ago
|
||
I can make an api for you.
Comment 10•13 years ago
|
||
Andreas, that would be great. Could you post a patch to this bug perhaps? I could at least play with such API a bit to see if we can improve responsiveness. I'm also trying to figure out how the MaybeGC in nsJSContext::ScriptEvaluated works with MaybeGC call in nsJSContext::DOMOperationCallback. If it was just documented somewhere when OperationCallback is actually called...
Comment 11•13 years ago
|
||
Just trying out something; sunspider seems to trigger compartment gc occasionally, but for some reason FOTN doesn't (or if it does, it happens very rarely). Posted to tryserver to see if this regresses sunspider. humph, want to try FOTN with this?
Comment 12•13 years ago
|
||
Patrick, do you have any other ideas how to fix this - I mean, how to cause less GC, or faster GC?
Comment 13•13 years ago
|
||
smaug, break on js_GC and walk up the stack to see what heuristic triggered that GC. That should tell us why it doesn't hit faster compartment GCs.
Comment 14•13 years ago
|
||
I ran Olli's try server build and compared to trunk with FOTN, and on trunk I see micro-pauses every so often, which I assume are gc. With this build I got at least 6 world-stopping pauses that lasted a second or so, and really froze the main thread. I also tried on our new demo, but it has pretty different run-time characteristics than FOTN. In it I noticed a few large pauses I didn't recognize, too, but I'm less confident to say that they are the result of this patch.
Comment 15•13 years ago
|
||
(In reply to comment #14) > I ran Olli's try server build and compared to trunk with FOTN, and on trunk I > see micro-pauses every so often, which I assume are gc. With this build I got > at least 6 world-stopping pauses that lasted a second or so, and really froze > the main thread. Um, that sounds bad. Andreas, Patrick, do you have any ideas how to improve gc scheduling without regressing perf tests?
Updated•13 years ago
|
Whiteboard: [softblocker]
Comment 16•13 years ago
|
||
Any GC differences with Ubuntu? I'm seeing horrible GC pauses still with my own projects specifically unique to Ubuntu (10.10 x64).
Reporter | ||
Comment 17•13 years ago
|
||
(In reply to comment #16) > Any GC differences with Ubuntu? I'm seeing horrible GC pauses still with my own > projects specifically unique to Ubuntu (10.10 x64). The code is the same across platforms. Are you generating a lot of garbage? There isn't much we can do at the moment if you're actually generating a lot of garbage. Incidentally, this bug is largely resolved for me with Andreas' patch to revamp the GC heuristics.
Comment 18•13 years ago
|
||
I think my GC problem that only happens with Firefox 4 on Ubuntu 10.10 x64 is related to scaling the canvas. I think there's some serious GC problem with nearest-neighbor scaling via the Mozilla CSS extension -moz-crisp-edges unique to Firefox on Ubuntu.
Comment 19•13 years ago
|
||
Here's the trouble line in my CSS file: https://github.com/grantgalitz/GameBoy-Online/blob/master/css/GameBoy.css.php#L260
Can you file a separate bug on that? It's unlikely to be related to this.
Comment 21•13 years ago
|
||
** PRODUCT DRIVERS PLEASE NOTE ** This bug is one of 19 being moved from blocking2.0:betaN+ to blocking2.0:- as we reached the endgame of Firefox 4. The rationale for the move is: - the bug had been identified as a "soft" blocker which could be fixed in some follow up release - the bug had been identified as one requiring beta coverage, thus is not appropriate for a ".x" stability & security release The owner of the bug may wish to renominate for .x consideration.
blocking2.0: betaN+ → .x+
Comment 23•13 years ago
|
||
Comment on attachment 502945 [details] [diff] [review] Proposed patch. Was backed out, thus is no longer approved!
Attachment #502945 -
Flags: approval2.0+ → approval2.0-
Comment 24•13 years ago
|
||
I wanted to land this bug on cedar, but it was not obvious which patch(es) should land. Please submit a patch which is ready to land on cedar (or push there yourself directly).
Updated•13 years ago
|
Whiteboard: [softblocker] → [softblocker][not-ready]
Assignee | ||
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
Comment 25•2 years ago
|
||
The bug assignee is inactive on Bugzilla, so the assignee is being reset.
Assignee: pwalton → nobody
Comment 26•2 years ago
|
||
GC scheduling has changed a lot in the last 11 years. The patch removes some variable mNumEvaluations which doesn't seem to exist any more.
Status: REOPENED → RESOLVED
Closed: 13 years ago → 2 years ago
Resolution: --- → INCOMPLETE
You need to log in
before you can comment on or make changes to this bug.
Description
•