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

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

[Jon Coppeard (:jonco)]

Attachment: relax-gray-asserts

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.

Comment 1

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8831760 [details] [diff] [review]
relax-gray-asserts

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

I had to think through this for a while, but it makes sense.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/efe0d03a98f1
Don't report that cells are gray in uncollected zones during incremental GC r=sfink

Comment 3

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/efe0d03a98f1

Comment 4

[Ryan VanderMeulen [:RyanVM]]

Please request Aurora/Beta approval on this when you get a chance.

Comment 5

[Jon Coppeard (:jonco)]

Comment on attachment 8831760 [details] [diff] [review]
relax-gray-asserts

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.

Comment 6

[Gerry Chang [:gchang]]

Comment on attachment 8831760 [details] [diff] [review]
relax-gray-asserts

Fix an issue related to GC. Aurora53+.

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/2e221577245f

Comment 8

[Julien Cristau [:jcristau]]

Comment on attachment 8831760 [details] [diff] [review]
relax-gray-asserts

gc fix for beta52

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/1bd7bd27d4cf

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/1bd7bd27d4cf