Closed Bug 1335117 Opened 5 years ago Closed 5 years ago

Don't report that cells are gray in uncollected zones during incremental GC


(Core :: JavaScript: GC, defect)

Not set



Tracking Status
firefox52 --- fixed
firefox-esr52 --- fixed
firefox53 --- fixed
firefox54 --- fixed


(Reporter: jonco, Assigned: jonco)




(1 file)

At the moment we unmark gray cells that are the target of a black cross compartment wrapper when they are in an uncollected zone (see ShouldTraceCrossCompartment / GCRuntime::foundBlackGrayEdges / AutoExposeLiveCrossZoneEdges).

The problem with this is that if a cross compartment wrapper is marked due to a barrier then this will not happen until the next GC slice runs and the mutator can observe gray to black edges.  This is OK since we only need to prevent the CC from observing such edges, but it does mean we can get spurious assertion failures related to gray marking.

This patch causes us to not report cells as gray in this situation.  Hopefully this will reduce the number of gray marking assertion failures we see.

I'm also working on adding a full heap trace to verify there are no black->gray edges that will run just before the CC.
Attachment #8831760 - Flags: review?(sphink)
Comment on attachment 8831760 [details] [diff] [review]

Review of attachment 8831760 [details] [diff] [review]:

I had to think through this for a while, but it makes sense.
Attachment #8831760 - Flags: review?(sphink) → review+
Pushed by
Don't report that cells are gray in uncollected zones during incremental GC r=sfink
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Please request Aurora/Beta approval on this when you get a chance.
Flags: needinfo?(jcoppeard)
Comment on attachment 8831760 [details] [diff] [review]

Approval Request Comment
[Feature/Bug causing the regression]: Incremental GC.
[User impact if declined]: No user impact, but possible spurious assertion failures in testing
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: It's a simple change to report GC things as gray less often.  
[String changes made/needed]: None.
Flags: needinfo?(jcoppeard)
Attachment #8831760 - Flags: approval-mozilla-beta?
Attachment #8831760 - Flags: approval-mozilla-aurora?
Comment on attachment 8831760 [details] [diff] [review]

Fix an issue related to GC. Aurora53+.
Attachment #8831760 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
See Also: → 1336630
Comment on attachment 8831760 [details] [diff] [review]

gc fix for beta52
Attachment #8831760 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Depends on: 1346417
Depends on: 1352373
See Also: → 1359231
You need to log in before you can comment on or make changes to this bug.