Closed
Bug 1105089
Opened 9 years ago
Closed 9 years ago
Always use at minimum 4 slices for iCC?
Categories
(Core :: XPCOM, defect)
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: smaug, Assigned: smaug)
Details
Attachments
(1 file, 1 obsolete file)
8.98 KB,
patch
|
mccr8
:
review+
|
Details | Diff | Splinter Review |
Currently http://mxr.mozilla.org/mozilla-central/source/xpcom/base/nsCycleCollector.cpp?rev=91e6e73aa5e6#3627 relies on budget to be over, but could we reduce the risk for over budget by actually always run IdlePhase, GraphBuildingPhase+, ScanAndCollectWhitePhase, CleanupPhase Currently it is possible that GraphBuildingPhase takes almost 5ms, and then we enter to unlinking which runs sync and takes quite some time. (but getting late now or so, since I don't now see what guarantees sane slices in http://mxr.mozilla.org/mozilla-central/source/dom/base/nsJSEnvironment.cpp?rev=683c4915bcee#1692)
Assignee | ||
Comment 1•9 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #0) > (but getting late now or so, since I don't now see what guarantees sane > slices in > http://mxr.mozilla.org/mozilla-central/source/dom/base/nsJSEnvironment. > cpp?rev=683c4915bcee#1692) Yes, just too late here :)
Comment 2•9 years ago
|
||
This logic is supposed to do something along those lines: http://mxr.mozilla.org/mozilla-central/source/xpcom/base/nsCycleCollector.cpp#3637
Assignee | ||
Comment 3•9 years ago
|
||
continueSlice = aBudget.isUnlimited() || mResults.mNumSlices < 3; is the part I was wondering. Why mResults.mNumSlices < 3? And also forcing GraphBuildingPhase to be always in different slice than ScanAndCollectWhitePhase.
Assignee | ||
Comment 4•9 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=3290ff1ebd45 I was thinking something like this. During games and such, we should try to have as short slices as possible so that we don't take too much time of the 16ms slice.
Assignee | ||
Comment 5•9 years ago
|
||
Maybe we want to have this behavior only if we're painting all the time, and otherwise keep the old behavior or something. (though, 4 slices isn't that bad)
Comment 6•9 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #5) > Maybe we want to have this behavior only if we're painting all the time, and > otherwise keep the old behavior or something. (though, 4 slices isn't that bad) Yeah, I think that makes sense. With 4 slices we end up taking a lot longer, so it isn't very power efficient, but that's a minor issue in the grand scheme of things.
Assignee | ||
Comment 7•9 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=72ea97a67693 In FlightOfTheNavigator we get usually 4 slices which are very short, but normal browsing can then have longer but fewer pauses.
Assignee: nobody → bugs
Attachment #8529023 -
Attachment is obsolete: true
Attachment #8529315 -
Flags: review?(continuation)
Comment 8•9 years ago
|
||
Comment on attachment 8529315 [details] [diff] [review] only 4+ slices when we're painting something Review of attachment 8529315 [details] [diff] [review]: ----------------------------------------------------------------- Thanks! ::: xpcom/base/nsCycleCollector.cpp @@ +3601,5 @@ > bool > nsCycleCollector::Collect(ccType aCCType, > SliceBudget& aBudget, > + nsICycleCollectorListener* aManualListener, > + bool aPreferShorterSlices) Maybe put this argument after the budget argument, as it is kind of related to the budget? @@ +3642,5 @@ > // (There's no need to check if we've finished graph building, because > // if we haven't, we've already exceeded our budget, and will finish > // this slice anyways.) > + continueSlice = aBudget.isUnlimited() || > + (mResults.mNumSlices < 3 && !aPreferShorterSlices); This is a little awkward, but ok.
Attachment #8529315 -
Flags: review?(continuation) → review+
Assignee | ||
Comment 9•9 years ago
|
||
(In reply to Andrew McCreight [:mccr8] from comment #8) > > Maybe put this argument after the budget argument, as it is kind of related > to the budget? Well, I wanted to have the default value for aPreferShorterSlices ( = false).
Assignee | ||
Comment 10•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/7f94a959f345
Comment 11•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7f94a959f345
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in
before you can comment on or make changes to this bug.
Description
•