958353 - Add some way to run slices of ICC for fuzzing and test cases

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Comment 1

[Andrew McCreight [:mccr8]]

My current plan is to add two methods to Components.utils (and then to SpecialPowers)

finishCC(): If any incremental CC is currently running, finish it.  This is necessary for creating test cases that can be run on TBPL when all sorts of stuff might be happening.

ccSlice(n): This does |n| units of work, where scanning a single object is some small number of units of work, like 1 to 5.  If no ICC is in progress, it begins one.

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: part 1 - Add finishCC() and ccSlice() methods for testing incremental cycle collection.

Note that this will do incremental collections even if ICC is disabled, so if it
gets globally disabled, we still are running any ICC tests.

This also changes a test to use these new functions, to make the intermittent
failure in bug 981033 permaorange. I confirmed that this test always fails without
the patch in that bug, and is green with it. I'll see if I can think of any other
case where it would be useful to add to a test. (A similar test could be done for
listeners, but it is the same issue, so I'm not too motivated to actually write
a test.)

I'll do a try run when the try server isn't hosed, but I think this is low risk.

Comment 3

[Andrew McCreight [:mccr8]]

Comment on attachment 8415524 [details] [diff] [review]
part 1 - Add finishCC() and ccSlice() methods for testing incremental cycle collection.

Jesse, does this look reasonable to you?  If you'd like, you should be able to start fuzzing with this patch applied locally and those two methods added. I think Olli is on holiday for a day or two, so it will be a few days before this lands. Of course, feel free to just wait.

Comment 4

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

Comment on attachment 8415524 [details] [diff] [review]
part 1 - Add finishCC() and ccSlice() methods for testing incremental cycle collection.

>     /*
>      * To be called from JS only.
>      *
>+     * If any incremental CC is in progress, finish it. For testing.
>+     */
>+    void finishCC();
>+
>+    /*
>+     * To be called from JS only.
>+     *
>+     * Do some cycle collector work, with the given work budget.
>+     * For testing.
>+     */
>+    void ccSlice(in long long budget);
This could use some better documentation. What is a work budget? Is 1000 a lot or 1000000?

Comment 5

[Andrew McCreight [:mccr8]]

Yeah, that's a reasonable point.  I can point out the computation of budget-per-object, and I guess even just give the current values, though that makes me wary of falling out of date if something changes.

Comment 6

[Andrew McCreight [:mccr8]]

try run: https://tbpl.mozilla.org/?tree=Try&rev=1f448c344b35

I updated the comment.  And as you saw, I read over the code and figured out that the WorkBudget() stuff isn't quite right, so I filed some bugs to look at that.  It should still be a decent first step for fuzzing.

https://hg.mozilla.org/integration/mozilla-inbound/rev/04589e152a80

Comment 7

[Andrew McCreight [:mccr8]]

I had to disable the changes to the test.  It turns out this isn't quite ready for use, or for fuzzing: if an incremental GC is in progress, and you start up a incremental CC, you hit an assert.  I think the call into the CC just needs to finish off any incremental GCs in progress, like we do for any time we call the CC from nsJSEnvironment.

https://hg.mozilla.org/integration/mozilla-inbound/rev/a3ced31038ba

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/04589e152a80
https://hg.mozilla.org/mozilla-central/rev/a3ced31038ba

Comment 9

[Andrew McCreight [:mccr8]]

Attachment: part 2 - Always check if an ICC is in progress in FixWeakMappingGrayBits().

Before we'd only check there was no ICC in progress when we actually had a weak mapping, but I think in general
we want to check it all the time when we begin a GC.  This makes the test case in the next part permaorange,
without the fix to the ccSlice() callback to finish off any GCs that are running.

Ideally, we'd check this every CC slice in case something is weird with the logic that stops a CC when a
GC begins, but that would require more work so I'll just leave it alone for now.

try run: https://tbpl.mozilla.org/?tree=Try&rev=2f639b629bef

Comment 10

[Andrew McCreight [:mccr8]]

Attachment: part 3 - Finish any IGC in progress when manually triggering an ICC slice.

Comment 11

[Andrew McCreight [:mccr8]]

Comment on attachment 8421788 [details] [diff] [review]
part 3 - Finish any IGC in progress when manually triggering an ICC slice.

Oops, forgot to flag this for review, too.

Comment 12

[Andrew McCreight [:mccr8]]

Thanks for the reviews.

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/4991012c3855
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/a3333ad4b1b2

Comment 13

[Andrew McCreight [:mccr8]]

This now has a test for:
1. Starting an ICC slice in the middle of an IGC.
2. Running a sync CC in the middle of an ICC.

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4991012c3855
https://hg.mozilla.org/mozilla-central/rev/a3333ad4b1b2