1368069 - Drop the fast GC slice budget to 5ms in Nightly

Status: RESOLVED DUPLICATE of bug 1367905

(Core :: JavaScript: GC, enhancement)

Description

[Steve Fink [:sfink] [:s:]]

We would like to try setting the slice duration to 5ms to see what happens. We can back it out if it causes issues.

I *think* the expected effect would be:

1. smoother operation (fewer missed frames) during the incremental marking portion. (I don't know what telemetry would indicate this, but would like to know.)

2. no effect on the mark -> sweep or other slow transition pauses. (Visible in telemetry via GC_MAX_PAUSE_MS.)

3. more long pauses from nonincremental GCs that happen because we decided we desperately needed to GC while the incremental GC was still ongoing. (Visible in telemetry via the GC_NON_INCREMENTAL proportion.)

4. slower mutator (non-GC) activity for longer periods of time while incremental collection is ongoing, having downstream effects. (Perhaps visible in benchmarks?)

So the question at hand is at what point #3 and #4 outweigh #1.

Comment 1

[Andrew McCreight [:mccr8]]

One approach to think about is what I did for the CC in bug 1304205: I increase the slice duration the longer the CC goes on, to try to avoid (3). This didn't have any visible effect on max pause in telemetry (maybe due to bad CC times being so long tail they are hard to pick out), but it did reduce the length of the 95th percentile of CCs by about a third.

Comment 2

[Paul Bone [:pbone]]

Attachment: Set the minimum GC slice time to 5ms

This passes try (enough):

https://treeherder.mozilla.org/#/jobs?repo=try&revision=182d79ef9d3524a7a22c46f869242dbc6e58870d&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=runnable

We should leave the bug open so that we can see the results in telemetry.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

FWIW I was about to reduce the time to 5ms too in my patches to use idle time more.
(need to fix something in TimerThread before getting back to the idle patches.)

Comment 4

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8871934 [details] [diff] [review]
Set the minimum GC slice time to 5ms

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

It seems ok to try this out for now. I think we probably ought to implement mccr8's suggestion eventually. Olli, let us know if this is going to interfere with your idle stuff.

Requesting feedback from jonco to verify that he's ok with landing this.

Comment 5

[Jon Coppeard (:jonco)]

Comment on attachment 8871934 [details] [diff] [review]
Set the minimum GC slice time to 5ms

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

How are we going to measure if this is successful / causes problems?  As long as we have a plan written down then I'm happy with trying this.

Comment 6

[Jon Coppeard (:jonco)]

I filed bug 1368972 for mccr8's suggestion.

Comment 7

[Jean Gong :jgong]

Kannan, 

Just a follow up from our QF triage last week.  Suggestion on next step to get this bug moving since Jon's filing bug 1368972 based on mccr8's suggestion.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

FWIW, same change was in my patch for bug 1367905.
And in case we use timers, the slice is at minimum pref * 2 (10ms) and max 50ms (since 50ms is max idle period)

https://hg.mozilla.org/mozilla-central/file/981da978f1f686ad024fa958c9d27d2f8acc5ad0/modules/libpref/init/all.js#l1423

Comment 9

[Steve Fink [:sfink] [:s:]]

Yeah, this has been obsoleted by bug 1367905, so we're good now.