774104 - Miscellaneous incremental GC fixes

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

Found while debugging tests...

Comment 1

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

Attachment: avoid leaks during JS_TransplantObject

This fixes a problem where we leak all compartments if an incremental GC is happening during a transplant. The transplant creates an object in every compartment, even ones that are no longer reachable, and incremental GC treats all objects allocated during the GC as live. This patch just finishes any GCs before the transplant.

Comment 2

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

Attachment: call purge at start of incremental GC

Every compartment has a single arena for each alloc kind that is the one it's currently allocating into. At the end of each GC slice, we schedule objects in these arenas to be marked, in case any additional allocation happens in them.

This patch flushes out these arenas at the start of the incremental GC. This way, if a compartment is dead, we won't keep alive the arenas that it last allocated into.

Comment 3

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

Attachment: put a read barrier on rooted objects

Sometimes the JS engine turns weak references into strong references by rooting the given object. We need a read barrier when this happens, since the object might not have been traced otherwise, but it should be now that it's a strong reference. This patch adds the barrier to the code in the JS engine that adds roots.

Comment 4

[Andrew McCreight [:mccr8]]

Comment on attachment 642410 [details] [diff] [review]
avoid leaks during JS_TransplantObject

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

Looks good to me, though it would be good if you did a little bit of browsing around to make sure it doesn't totally hork up page navigation performance.

Comment 5

[Andrew McCreight [:mccr8]]

Comment on attachment 642411 [details] [diff] [review]
call purge at start of incremental GC

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

Bill explained in IRC that this could cause us to use more arenas, because if you may end up creating a new arena where you could have just used extra space in the arena you purged, but after the GC is over, the partially filled arena will be used for allocation again.  So hopefully it won't cause too much fragmentation.

Comment 6

[Andrew McCreight [:mccr8]]

Comment on attachment 642412 [details] [diff] [review]
put a read barrier on rooted objects

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

Thanks for the explanations for all of these patches, they were very helpful.

Comment 7

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/8c929d90f925
https://hg.mozilla.org/integration/mozilla-inbound/rev/2a2f3c7196fb
https://hg.mozilla.org/integration/mozilla-inbound/rev/993d0060a5a5

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8c929d90f925
https://hg.mozilla.org/mozilla-central/rev/2a2f3c7196fb
https://hg.mozilla.org/mozilla-central/rev/993d0060a5a5