1897473 - Simplify GC zeal implementation and add tests

Status: RESOLVED FIXED

(Core :: JavaScript: GC, task)

Description

[Jon Coppeard (:jonco)]

While looking at bug 1897013 I wrote some tests that GC zeal was doing what I expected and found some cases where it wasn't so I wrote some patches to simplify the implementation and generally tidy up.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: Bug 1897473 - Part 1: Move GC zeal method definitions out of GCRuntime.h r?sfink

There's no need for these to be defined inline. They're mostly called from
GC.cpp where they are defined and the compiler will inline these automatically.

I also made methods const where possible.

Comment 2

[Jon Coppeard (:jonco)]

Attachment: Bug 1897473 - Part 2: Refactor zeal mode checks to use a more declarative approach r?sfink

This replaces use of large conditionals with defined sets of zeal modes. This
removes duplication between TwoSlicesZealModes and hasTwoSliceZealMode() which
were actually subtly different.

Comment 3

[Jon Coppeard (:jonco)]

Attachment: Bug 1897473 - Part 3: Make all periodic zeal modes mutually exclusive r?sfink

This makes more modes mutually exclusive. For modes that trigger GC I don't
think this makes any difference.

It does mean that we don't allow the barrier verifier to run at the same time
as a GC triggering zeal mode, but that doesn't make sense anyways since the
barrier verifier do anything if it's triggered during an incremental GC.

Comment 4

[Jon Coppeard (:jonco)]

Attachment: Bug 1897473 - Part 4: Don't schedule a GC for zeal modes that don't trigger GC r?sfink

This refines the check for which modes to schedule a GC for when enabling a zeal mode.

Comment 5

[Jon Coppeard (:jonco)]

Attachment: Bug 1897473 - Part 5: Turn off clang formatting for GC zeal help text r?sfink

Comment 6

[Jon Coppeard (:jonco)]

Attachment: Bug 1897473 - Part 6: Fix descriptions of zeal modes that trigger a slice per zone r?sfink

A few of the zeal modes trigger a slice per collected zone but are described as
triggering two slices. This does some renaming and updates the help text.
Better name suggestions are welcome.

Comment 7

[Jon Coppeard (:jonco)]

Attachment: Bug 1897473 - Part 7: Add tests for GC zeal r?sfink

This tests the number of GCs and slices triggered by the different zeal modes.

Added a way of exposing the slice number via a GC param, in the same way we do
for major and minor GC number.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3bbadb6f06e7
Part 1: Move GC zeal method definitions out of GCRuntime.h r=sfink
https://hg.mozilla.org/integration/autoland/rev/2c54b7e37a85
Part 2: Refactor zeal mode checks to use a more declarative approach r=sfink
https://hg.mozilla.org/integration/autoland/rev/309797fd703a
Part 3: Make all periodic zeal modes mutually exclusive r=sfink
https://hg.mozilla.org/integration/autoland/rev/428ddb1ae698
Part 4: Don't schedule a GC for zeal modes that don't trigger GC r=sfink
https://hg.mozilla.org/integration/autoland/rev/0daad850365a
Part 5: Turn off clang formatting for GC zeal help text r=sfink
https://hg.mozilla.org/integration/autoland/rev/d0d44044ef70
Part 6: Fix descriptions of zeal modes that trigger a slice per zone r=sfink
https://hg.mozilla.org/integration/autoland/rev/1502bfb7dcc2
Part 7: Add tests for GC zeal r=sfink

Comment 9

[Narcis Beleuzu [:NarcisB]]

Backed out for SM bustages on gczeal.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/33e67eb95f82b3356f8af555439cd079b8f844fc
Log link: https://treeherder.mozilla.org/logviewer?job_id=458967859&repo=autoland&lineNumber=58163

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9db2e18fef43
Part 1: Move GC zeal method definitions out of GCRuntime.h r=sfink
https://hg.mozilla.org/integration/autoland/rev/c290c8bea006
Part 2: Refactor zeal mode checks to use a more declarative approach r=sfink
https://hg.mozilla.org/integration/autoland/rev/6a2929944007
Part 3: Make all periodic zeal modes mutually exclusive r=sfink
https://hg.mozilla.org/integration/autoland/rev/ee72b307892f
Part 4: Don't schedule a GC for zeal modes that don't trigger GC r=sfink
https://hg.mozilla.org/integration/autoland/rev/0ce6e4ea9e62
Part 5: Turn off clang formatting for GC zeal help text r=sfink
https://hg.mozilla.org/integration/autoland/rev/da50ecbefd94
Part 6: Fix descriptions of zeal modes that trigger a slice per zone r=sfink
https://hg.mozilla.org/integration/autoland/rev/506670e0890f
Part 7: Add tests for GC zeal r=sfink

Comment 11

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/9db2e18fef43
https://hg.mozilla.org/mozilla-central/rev/c290c8bea006
https://hg.mozilla.org/mozilla-central/rev/6a2929944007
https://hg.mozilla.org/mozilla-central/rev/ee72b307892f
https://hg.mozilla.org/mozilla-central/rev/0ce6e4ea9e62
https://hg.mozilla.org/mozilla-central/rev/da50ecbefd94
https://hg.mozilla.org/mozilla-central/rev/506670e0890f