Closed Bug 911246 Opened 11 years ago Closed 10 years ago

Enable incremental cycle collection by default

Categories

(Core :: XPCOM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla32
Tracking Status
firefox31 --- disabled
firefox32 + disabled
firefox33 --- fixed

People

(Reporter: mccr8, Assigned: mccr8)

References

(Blocks 1 open bug)

Details

(Whiteboard: [Snappy])

Attachments

(2 files, 1 obsolete file)

ICC is going to land preffed off by default.
This is seriously exciting. ++Andrew
Depends on: 948755
For convenience for anybody who wants to try it when it is ready to be turned on,
here's a patch that will enable it.  It won't do anything yet.
Depends on: 927601
Depends on: 937826
Depends on: 949607
Depends on: 950791
Depends on: 950949
Depends on: 950959
Depends on: 951796
Depends on: 948554
Blocks: 956068
Depends on: 957370
Depends on: 957416
Attachment #8346014 - Attachment is obsolete: true
No longer depends on: 950791
Well, BC is quite orange (about 80% of the time).  I'll have to look into those failures.
Depends on: 963844
No longer depends on: 927601, 937826, 957416
Depends on: 975503
Depends on: 977940
Depends on: 981033
Depends on: 957109
-p all try run is green: https://tbpl.mozilla.org/?tree=Try&rev=c137fc13f44d
XP M4 looks green: https://tbpl.mozilla.org/?tree=Try&rev=98d6571b6f4c

I'm just waiting on review for one more patch blocking this bug.
Comment on attachment 8357210 [details] [diff] [review]
Enable incremental cycle collection by default.

Bug 996151 has landed, so this should be ready to land now.
Attachment #8357210 - Flags: review?(bugs)
Comment on attachment 8357210 [details] [diff] [review]
Enable incremental cycle collection by default.

woohoo
Attachment #8357210 - Flags: review?(bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/e4be5203a3c9
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Do you want to uplift this so it goes into 31ESR? Or did this land after 31 branching intentionally?
Flags: needinfo?(continuation)
There's no particular reason it needs to go into ESR31.
Flags: needinfo?(continuation)
Depends on: 1005253
Depends on: 1005278
Depends on: 1005963
It looks like this reduced the 95th percentile of CYCLE_COLLECTOR_MAX_PAUSE from 82ms to around 40ms, and the 75th percentile from 26ms to 13ms.

http://telemetry.mozilla.org/#nightly/32/CYCLE_COLLECTOR_MAX_PAUSE
No longer depends on: 1005963
Depends on: 1007828
Depends on: 1023758
Depends on: 1011391
Comment on attachment 8444597 [details] [diff] [review]
Backout bug 911246 from 32 for crashes.

r+ for Aurora. I think we should not disable icc on trunk.
Attachment #8444597 - Flags: review?(bugs) → review+
Comment on attachment 8444597 [details] [diff] [review]
Backout bug 911246 from 32 for crashes.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 911246
User impact if declined: crashes
Testing completed (on m-c, etc.): this puts us back to the state we're in on beta
Risk to taking this patch (and alternatives if risky): should be low, we're just reverted to what we did before
String or IDL/UUID changes made by this patch: none
Attachment #8444597 - Flags: approval-mozilla-aurora?
Comment on attachment 8444597 [details] [diff] [review]
Backout bug 911246 from 32 for crashes.

This approval request appears to have gotten lost, but I have a work in progress patch for the crash, so I'll skip this for now.
Attachment #8444597 - Flags: approval-mozilla-aurora?
Comment on attachment 8444597 [details] [diff] [review]
Backout bug 911246 from 32 for crashes.

I have a patch in progress that fixes ICC, but it is getting a little tricky, so I think it is better to just disable ICC on Aurora and let my ICC fix ride the trains.

Approval Request Comment: See comment 16. This should fix top crashes on Aurora.
Attachment #8444597 - Flags: approval-mozilla-aurora?
Comment on attachment 8444597 [details] [diff] [review]
Backout bug 911246 from 32 for crashes.

Approved for Aurora. I look forward to watching ICC ride the 33 train.
Attachment #8444597 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Keywords: checkin-needed
Whiteboard: [Snappy] → [Snappy][checkin Aurora]
You need to log in before you can comment on or make changes to this bug.