1630961 - GC scheduling and sweeping tidyups

Status: RESOLVED FIXED

(Core :: JavaScript: GC, task, P1)

Description

[Jon Coppeard (:jonco)]

In preparation for tackling bug 1627039, here are some scheduling and sweeping tidyups.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: Bug 1630961 - Remove INCREMENTAL_TOO_SLOW GC trigger as this case is also handled elsewhere r?sfink

This is checkd in the allocator on every allocation, but we also check this when we allocate a new arena in maybeAllocTriggerZoneGC. It's possible that this one will hit first if background sweeping reduces the heap threshold after the last arena was allocated, but this doesn't really make any difference.

Comment 2

[Jon Coppeard (:jonco)]

Attachment: Bug 1630961 - Remove unused heurisitic to delaying GCs that may cause resets r?sfink

A while back we added a heuristic to delay a GC if it would cause a reset (bug 1367455), but it was turned off after it caused a performance regression and hasn't been reenabled since.

Telemetry shows that that only 0.2% of GCs get reset anyway so we should probably just remove this.

Depends on D71328

Comment 3

[Jon Coppeard (:jonco)]

Attachment: Bug 1630961 - Don't free LIFO data in background sweep task as this already happens in the background free task r?sfink

I think I meant to do this when I added the separate GCRuntime::freeTask, but as it stands both this and sweepTask try and free the LIFO data. This is unnecessary.

Depends on D71329

Comment 4

[Jon Coppeard (:jonco)]

Attachment: Bug 1630961 - Assert background sweeping has finished in incremental collections where possible r?sfink

The reason this didn't pass before is that the sweep task may be started for every group of zones, so a single instance finishing doesn't mean that we've finished sweeping all zones, and there may be more arenas to sweep in this case.

Depends on D71330

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4ec83f8bd505
Remove INCREMENTAL_TOO_SLOW GC trigger as this case is also handled elsewhere r=sfink
https://hg.mozilla.org/integration/autoland/rev/0aab8dc843cd
Remove unused heurisitic to delaying GCs that may cause resets r=sfink
https://hg.mozilla.org/integration/autoland/rev/b17ee0161ec7
Don't free LIFO data in background sweep task as this already happens in the background free task r=sfink
https://hg.mozilla.org/integration/autoland/rev/004bab704062
Assert background sweeping has finished in incremental collections where possible r=sfink

Comment 6

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/4ec83f8bd505
https://hg.mozilla.org/mozilla-central/rev/0aab8dc843cd
https://hg.mozilla.org/mozilla-central/rev/b17ee0161ec7
https://hg.mozilla.org/mozilla-central/rev/004bab704062