Closed
Bug 735099
Opened 13 years ago
Closed 12 years ago
Re-enable incremental GC on desktop platforms
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla16
People
(Reporter: billm, Assigned: billm)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
(Whiteboard: [k9o:p1:fx15][games:p1][Snappy])
Attachments
(1 file)
(deleted),
patch
|
terrence
:
review+
|
Details | Diff | Splinter Review |
Bug 734946 will disable incremental GC. This bug is to re-enable it and to fix the leaks that seem to show up when it's enabled during browser-chrome tests.
Updated•13 years ago
|
Blocks: IncrementalGC
Assignee | ||
Comment 1•13 years ago
|
||
Here is a recent try push for re-enabling. It still leaks.
https://tbpl.mozilla.org/?tree=Try&rev=59b6f9fff21d
It's possible that this is related to bug 723832, which shows a leak of a similar size happening several times per day. However, with incremental GC enabled we leak much more frequently.
Comment 2•13 years ago
|
||
The patch in bug 727965 makes incremental GC leak on Linux (32 or 64 or both?) Moth, or it did a few months ago. If it happens to be the same underlying issue, it might be easier to track down on Linux instead of XP.
I think I have reduced the leak to LayoutModuleDtor not being called. This try run should confirm it:
https://tbpl.mozilla.org/?tree=Try&rev=9c8bdd5e3ab8
Comment 4•13 years ago
|
||
Any verdict?
Updated•13 years ago
|
blocking-kilimanjaro: --- → ?
Assignee | ||
Comment 5•13 years ago
|
||
Bug 750424 fixes the leak. I want to take a look at some memory usage regressions that showed up the last time incremental was enabled. Otherwise, this is ready to go.
Comment 6•13 years ago
|
||
+ for Kilimanjaro. IGC is one of the key things we need for smooth game perf.
blocking-kilimanjaro: ? → +
Whiteboard: [k9o:p1:fx15]
Updated•13 years ago
|
Summary: Re-enable incremental GC → Re-enable incremental GC on Intel platforms
Comment 7•13 years ago
|
||
Why just Intel?
Comment 8•13 years ago
|
||
See bug 750959 for Android. It had a different problem blocking it from being turned on, or something like that.
Comment 9•13 years ago
|
||
Has a different problem, rather than had.
Assignee | ||
Updated•13 years ago
|
Summary: Re-enable incremental GC on Intel platforms → Re-enable incremental GC on desktop platforms
Comment 10•13 years ago
|
||
unfortunately Bill, i just logged bug 752191 which may delay incremental gc again :(
and it was going oh so well till now....
Updated•13 years ago
|
Whiteboard: [k9o:p1:fx15] → [k9o:p1:fx15][games:p1]
Assignee | ||
Comment 11•13 years ago
|
||
Attachment #623332 -
Flags: review?(terrence)
Updated•13 years ago
|
Attachment #623332 -
Flags: review?(terrence) → review+
Assignee | ||
Comment 12•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/1a9c5ac02776
Let's see if this sticks. There are a bunch of tuning fixes I want to get in, but I'd like to land this now so that other patches don't regress it.
Target Milestone: --- → mozilla15
Comment 13•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 14•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac968ff4fe41
https://hg.mozilla.org/mozilla-central/rev/ac968ff4fe41
I had to back this out for crashes. I think I know the cause, though.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla15 → ---
Comment 15•13 years ago
|
||
By the way, here's another intermittent crash that hadn't been filed yet:
https://tbpl.mozilla.org/php/getParsedLog.php?id=11717280&tree=Mozilla-Inbound
Rev4 MacOSX Snow Leopard 10.6 mozilla-inbound debug test mochitests-5/5 on 2012-05-13 08:43:26 PDT for push 5fbb80b8e84f
TEST-UNEXPECTED-FAIL | /tests/toolkit/components/passwordmgr/test/test_prompt_async.html | Exited with code 1 during test run
0 XUL!js::gc::GetGCThingTraceKind [Heap.h : 497 + 0x1c]
rbx = 0x0756d000 r12 = 0x00000000 r13 = 0x04a94230 r14 = 0x00000000
r15 = 0xffffffff rip = 0x035512ce rsp = 0x5fbfc9d0 rbp = 0x5fbfc9e0
Found by: given as instruction pointer in context
1 XUL!js::gc::MarkKind [Marking.cpp : 230 + 0x4]
rbx = 0x5fbfca28 r12 = 0x00000000 r13 = 0x04a94230 r14 = 0x00000000
r15 = 0xffffffff rip = 0x0354bcd7 rsp = 0x5fbfc9f0 rbp = 0x5fbfca10
Found by: call frame info
2 XUL!js::gc::MarkValueInternal [Marking.cpp : 329 + 0xb]
rbx = 0x0756d080 r12 = 0x092e57e8 r13 = 0x04a94230 r14 = 0x00000000
r15 = 0xffffffff rip = 0x0354c054 rsp = 0x5fbfca20 rbp = 0x5fbfca60
Found by: call frame info
3 XUL!proxy_TraceObject [jsproxy.cpp : 1259 + 0x19]
rbx = 0x092e57c0 r12 = 0x04a94230 r13 = 0x042ad140 r14 = 0x04a94000
r15 = 0x008f3600 rip = 0x03389177 rsp = 0x5fbfca70 rbp = 0x5fbfca80
Found by: call frame info
4 XUL!js::GCMarker::processMarkStackTop [Marking.cpp : 1092 + 0x9]
rbx = 0x1ca93fb0 r12 = 0x042b18a0 r13 = 0x042ad140 r14 = 0x04a94000
r15 = 0x008f3600 rip = 0x035582b4 rsp = 0x5fbfca90 rbp = 0x5fbfcb20
Found by: call frame info
5 XUL!js::GCMarker::drainMarkStack [Marking.cpp : 1136 + 0xa]
rbx = 0x04a94230 r12 = 0x5fbfcbb0 r13 = 0x04a94268 r14 = 0x04a94000
r15 = 0x008f3600 rip = 0x0354f78b rsp = 0x5fbfcb30 rbp = 0x5fbfcb50
Found by: call frame info
6 XUL!GCCycle [jsgc.cpp : 3471 + 0x19]
rbx = 0x008f3740 r12 = 0x008f3740 r13 = 0x00000002 r14 = 0x46876000
r15 = 0x008f3600 rip = 0x032a00f2 rsp = 0x5fbfcb60 rbp = 0x5fbfcc40
Found by: call frame info
7 XUL!Collect [jsgc.cpp : 3719 + 0x11]
rbx = 0x04a94320 r12 = 0x04a94000 r13 = 0x04a94b60 r14 = 0x00000001
r15 = 0x5fbfcc70 rip = 0x032a0a91 rsp = 0x5fbfcc50 rbp = 0x5fbfccb0
Found by: call frame info
8 XUL!nsJSContext::GarbageCollectNow [nsJSEnvironment.cpp : 2994 + 0xe]
rbx = 0x00000000 r12 = 0x00000000 r13 = 0x08298000 r14 = 0x0085d600
r15 = 0x00000018 rip = 0x01cab24b rsp = 0x5fbfccc0 rbp = 0x5fbfccf0
Found by: call frame info
9 XUL!nsTimerImpl::Fire [nsTimerImpl.cpp : 508 + 0xa]
Updated•13 years ago
|
Whiteboard: [k9o:p1:fx15][games:p1] → [k9o:p1:fx15][games:p1][Snappy]
Assignee | ||
Comment 16•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/a6be10cfee3f
To make this work I had to disable bug 716014. Hopefully we can fix that for the next cycle.
Target Milestone: --- → mozilla15
Comment 17•12 years ago
|
||
What's with the extra GCs and CCs in the explicit call to GarbageCollect()? Is that needed to make some test pass?
Updated•12 years ago
|
blocking-basecamp: --- → ?
tracking-firefox15:
--- → ?
Comment 18•12 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 13 years ago → 12 years ago
Resolution: --- → FIXED
Comment 20•12 years ago
|
||
I got a few upset mails from Talos when I merged this changeset (among others) to Fx-Team:
https://groups.google.com/d/topic/mozilla.dev.tree-management/BnQX5sR1M2Q/discussion
Looking at the graph server, this regression seems to have hit inbound around when this changeset landed and followed it to mozilla-central and eventually fx-team. I'm not sure why I don't see a tree-management mail before the fx-team merge.
There are a few other changesets in range, but this one seems like the prime suspect. Is this known?
Comment 21•12 years ago
|
||
(In reply to Dave Camp (:dcamp) from comment #20)
> Looking at the graph server, this regression seems to have hit inbound
> around when this changeset landed and followed it to mozilla-central and
> eventually fx-team. I'm not sure why I don't see a tree-management mail
> before the fx-team merge.
There were actually many regression mails when this first hit inbound and then central, although they were spread across several threads and some were mis-blamed. For example:
https://groups.google.com/d/topic/mozilla.dev.tree-management/oQ6HWhKoyQc/discussion
Comment 22•12 years ago
|
||
Comment 12 applies.
Comment 23•12 years ago
|
||
Why do we have to trigger multiple GCs now?
for (int i = 0; i < 3; i++) {
nsJSContext::GarbageCollectNow(js::gcreason::DOM_UTILS, nsGCNormal, true);
nsJSContext::CycleCollectNow(aListener, aExtraForgetSkippableCalls);
}
Comment 24•12 years ago
|
||
Ccing Olli Pettay to get his input on the above question.
Comment 25•12 years ago
|
||
I don't know why it was added.
It is not in the patch what was reviewed.
Assignee | ||
Comment 26•12 years ago
|
||
(In reply to Gregor Wagner [:gwagner] from comment #23)
> Why do we have to trigger multiple GCs now?
>
> for (int i = 0; i < 3; i++) {
> nsJSContext::GarbageCollectNow(js::gcreason::DOM_UTILS, nsGCNormal,
> true);
> nsJSContext::CycleCollectNow(aListener, aExtraForgetSkippableCalls);
> }
I was trying to resolve some orange that showed up on tryserver recently.
Comment 27•12 years ago
|
||
I also see regressions when running any benchmark or animation with more than one tab open. It seems we don't trigger compartment GCs any more.
My small netbook doesn't like that at all.
Comment 28•12 years ago
|
||
i just updated to http://hg.mozilla.org/mozilla-central/rev/dd6ec482a85d and am no longer getting any iGC at all, the value is true and about:support indicates it is enabled.
Comment 29•12 years ago
|
||
20120602134306
http://hg.mozilla.org/mozilla-central/rev/9274e6b53af4 is the most recent build i can get iGC to work
Comment 30•12 years ago
|
||
I just updated, and on my usual browsing set I'm getting all GCs as "Reason: CC_WAITING, Nonincremental Reason: request".
Updated•12 years ago
|
blocking-basecamp: ? → -
Updated•12 years ago
|
status-firefox15:
--- → disabled
Comment 32•12 years ago
|
||
How is this looking for mozilla16?
Comment 33•12 years ago
|
||
(In reply to Martin Best (:mbest) from comment #32)
> How is this looking for mozilla16?
It's on and looking good.
Updated•11 years ago
|
Blocks: gecko-games
You need to log in
before you can comment on or make changes to this bug.
Description
•