Closed Bug 958353 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: XPCOM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: mccr8, Assigned: mccr8)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

No description provided.
Assignee: nobody → continuation
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.
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.
Attachment #8415524 - Flags: review?(bugs)
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.
Attachment #8415524 - Flags: feedback?(jruderman)
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?
Attachment #8415524 - Flags: review?(bugs) → review+
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.
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
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
Keywords: leave-open
Attachment #8415524 - Attachment description: Add finishCC() and ccSlice() methods for testing incremental cycle collection. → part 1 - Add finishCC() and ccSlice() methods for testing incremental cycle collection.
Attachment #8415524 - Flags: checkin+
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
Attachment #8421786 - Flags: review?(bugs)
Attachment #8421786 - Flags: review?(bugs) → review+
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.
Attachment #8421788 - Flags: review?(bugs)
Attachment #8421788 - Flags: review?(bugs) → review+
Attachment #8415524 - Flags: feedback?(jruderman)
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.
Flags: in-testsuite+
Depends on: 1018394
Depends on: 1018397
You need to log in before you can comment on or make changes to this bug.