Closed Bug 735099 Opened 13 years ago Closed 12 years ago

Re-enable incremental GC on desktop platforms

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla16
blocking-kilimanjaro +
blocking-basecamp -
Tracking Status
firefox15 - disabled

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)

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.
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.
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
Any verdict?
blocking-kilimanjaro: --- → ?
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.
Depends on: 750424, 750416
+ for Kilimanjaro. IGC is one of the key things we need for smooth game perf.
blocking-kilimanjaro: ? → +
Whiteboard: [k9o:p1:fx15]
Depends on: 746253
Summary: Re-enable incremental GC → Re-enable incremental GC on Intel platforms
Why just Intel?
See bug 750959 for Android. It had a different problem blocking it from being turned on, or something like that.
Has a different problem, rather than had.
Summary: Re-enable incremental GC on Intel platforms → Re-enable incremental GC on desktop platforms
Depends on: 731423
Depends on: 752191
unfortunately Bill, i just logged bug 752191 which may delay incremental gc again :( and it was going oh so well till now....
No longer depends on: 752191
Depends on: 752098
Whiteboard: [k9o:p1:fx15] → [k9o:p1:fx15][games:p1]
Depends on: 752387
No longer blocks: 754303
Depends on: 752392
Attached patch patch (deleted) — Splinter Review
Attachment #623332 - Flags: review?(terrence)
Attachment #623332 - Flags: review?(terrence) → review+
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
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Depends on: 754588
Depends on: 754674
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla15 → ---
No longer depends on: 754588
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]
Depends on: 654903
Depends on: 719114
Depends on: 654196
Depends on: 654877
Whiteboard: [k9o:p1:fx15][games:p1] → [k9o:p1:fx15][games:p1][Snappy]
Depends on: 757483
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
What's with the extra GCs and CCs in the explicit call to GarbageCollect()? Is that needed to make some test pass?
blocking-basecamp: --- → ?
Status: REOPENED → RESOLVED
Closed: 13 years ago12 years ago
Resolution: --- → FIXED
No need to track - please keep the status flags in bug 641025 up to date.
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?
(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 12 applies.
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); }
Ccing Olli Pettay to get his input on the above question.
I don't know why it was added. It is not in the patch what was reviewed.
(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.
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.
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.
20120602134306 http://hg.mozilla.org/mozilla-central/rev/9274e6b53af4 is the most recent build i can get iGC to work
I just updated, and on my usual browsing set I'm getting all GCs as "Reason: CC_WAITING, Nonincremental Reason: request".
Depends on: 761739
Depends on: 762580
blocking-basecamp: ? → -
This was disabled again in 15.
Target Milestone: mozilla15 → mozilla16
How is this looking for mozilla16?
(In reply to Martin Best (:mbest) from comment #32) > How is this looking for mozilla16? It's on and looking good.
Blocks: gecko-games
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: