Closed Bug 774104 Opened 10 years ago Closed 10 years ago

Miscellaneous incremental GC fixes

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla16

People

(Reporter: billm, Assigned: billm)

References

Details

Attachments

(3 files)

Found while debugging tests...
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.
Attachment #642410 - Flags: review?(dvander)
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.
Attachment #642411 - Flags: review?(dvander)
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.
Attachment #642412 - Flags: review?(dvander)
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.
Attachment #642410 - Flags: review?(dvander) → review+
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.
Attachment #642411 - Flags: review?(dvander) → review+
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.
Attachment #642412 - Flags: review?(dvander) → review+
Blocks: 768716
Blocks: 771510
No longer blocks: 771510
Depends on: 774933
You need to log in before you can comment on or make changes to this bug.