Closed Bug 1487273 Opened 1 year ago Closed 1 year ago

Consider to add a limit how often forget skippable only cycles run

Categories

(Core :: XPCOM, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: smaug, Assigned: smaug)

Details

Attachments

(2 files)

nsJSContext::MaybePokeCC() uses NS_CC_SKIPPABLE_DELAY for IdleTaskRunner's timeout. I think we could increase that if sCleanupsSinceLastGC > NS_MAJOR_FORGET_SKIPPABLE_CALLS, since that means we've stopped the forget skippable run already at least once, and because GC hasn't run, we don't do new major runs, so forgetskippable is less important.
Priority: -- → P3
This might not help too much. We need a separate limit for forget skippable only cycles.
Re-purposing a bit.
Summary: Consider to increase NS_CC_SKIPPABLE_DELAY if GC hasn't run → Consider to add a limit how often forget skippable only cycles run
Something like this.
I wouldn't want to land this, or any changes to forget skippable scheduling to FF63, so whatever is done in this bug should land after 2018-09-04.
Given the telemetry I added, this could show frequency to drop.

remote: View the pushlog for these changes here:
remote:   https://hg.mozilla.org/try/pushloghtml?changeset=04f13a9e037f285c23227525f11513327e8d9f01
remote: 
remote: Follow the progress of your build on Treeherder:
remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=04f13a9e037f285c23227525f11513327e8d9f01
remote: recorded changegroup in replication log in 0.013s
Assignee: nobody → bugs
Attachment #9005738 - Flags: review?(continuation)
Comment on attachment 9005738 [details] [diff] [review]
forget_skippable_limit.diff

Review of attachment 9005738 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsJSEnvironment.cpp
@@ +119,5 @@
>  #define NS_CC_DELAY                 6000 // ms
>  
>  #define NS_CC_SKIPPABLE_DELAY       250 // ms
>  
> +// In case cycle collector isn't run at all, we don't want

nit: I think you should add "the" between "case" and "cycle".
Attachment #9005738 - Flags: review?(continuation) → review+
Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/634170c72bf7
add a limit how often forget skippable only cycles run, r=mccr8
https://hg.mozilla.org/mozilla-central/rev/634170c72bf7
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Target Milestone: mozilla64 → mozilla63
Target Milestone: mozilla63 → mozilla64
You need to log in before you can comment on or make changes to this bug.