Last Comment Bug 774104 - Miscellaneous incremental GC fixes
: Miscellaneous incremental GC fixes
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla16
Assigned To: Bill McCloskey (:billm)
: general
Mentors:
Depends on: 774933
Blocks: 768716
  Show dependency treegraph
 
Reported: 2012-07-15 13:06 PDT by Bill McCloskey (:billm)
Modified: 2012-08-02 16:34 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
avoid leaks during JS_TransplantObject (1.32 KB, patch)
2012-07-15 13:08 PDT, Bill McCloskey (:billm)
continuation: review+
Details | Diff | Review
call purge at start of incremental GC (1.11 KB, patch)
2012-07-15 13:11 PDT, Bill McCloskey (:billm)
continuation: review+
Details | Diff | Review
put a read barrier on rooted objects (2.19 KB, patch)
2012-07-15 13:13 PDT, Bill McCloskey (:billm)
continuation: review+
Details | Diff | Review

Description Bill McCloskey (:billm) 2012-07-15 13:06:39 PDT
Found while debugging tests...
Comment 1 Bill McCloskey (:billm) 2012-07-15 13:08:45 PDT
Created attachment 642410 [details] [diff] [review]
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 (:billm) 2012-07-15 13:11:58 PDT
Created attachment 642411 [details] [diff] [review]
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 (:billm) 2012-07-15 13:13:58 PDT
Created attachment 642412 [details] [diff] [review]
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] 2012-07-15 13:40:45 PDT
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] 2012-07-15 13:50:49 PDT
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] 2012-07-15 14:00:53 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.