817614 - GC: Fix minor issue with GC zeal mode and add tests

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jon Coppeard (:jonco)]

I started writing some tests to check that GC state changes as it should.  This showed up the problem that one of the zeal modes was yielding before entering the sweep phase, and this taking more than its supposed two slices.

This is not a big issue,

Comment 1

[Jon Coppeard (:jonco)]

the main idea is to start added tests for this stuff.

Comment 2

[Jon Coppeard (:jonco)]

The issue is with mode 8, which is meant to mark the roots in the first slice and then finish collecting in the second.  Instead, it adds and extra marking slice between the two.

Comment 3

[Jon Coppeard (:jonco)]

Attachment: Add test functions and fix zeal issue

I had to change GCDebugSlice to use DEBUG_GC reason, otherwise it's not possible to use the zeal modes from the shell like this.

Comment 4

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 687806 [details] [diff] [review]
Add test functions and fix zeal issue

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

Thanks. I like the idea of testing that GC zeal actually works.

::: js/src/builtin/TestingFunctions.cpp
@@ +462,5 @@
> +    gc::State globalState = cx->runtime->gcIncrementalState;
> +    if (globalState == gc::NO_INCREMENTAL)
> +        state = 0;
> +    else if (globalState == gc::MARK)
> +        state = 1;

Can you use strings here instead of numbers? You can generate the strings like so:
  *vp = js_NewStringCopyZ(cx, "mark");

::: js/src/jsgc.cpp
@@ +3797,5 @@
>  
>      if (zeal == ZealIncrementalRootsThenFinish || zeal == ZealIncrementalMarkAllThenFinish) {
>          /*
>           * Yields between slices occurs at predetermined points in these
> +         * modes, the budget is not used.

This should be a semicolon instead of a comma because it's an independent clause.

Comment 5

[Jon Coppeard (:jonco)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/115030ad174d

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/115030ad174d