Closed Bug 1286833 Opened 3 years ago Closed 3 years ago

Minor tweaks to improve tenuring performance

Categories

(Core :: JavaScript: GC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: jonco, Assigned: jonco)

Details

Attachments

(1 file)

Attached patch improve-tenuringSplinter Review
I was looking at how we tenure objects and came up with a couple of minor tweaks to improve this:

 - update the unique ID with new pointer when we sweep the nursery ID table rather than doing a lookup for every object we tenure
 - restructure the loop in StoreBuffer::traceWholeCells to take account of the fact that all cells in an arena have the same alloc kind

Probably only minimal performance gain though.
Attachment #8770950 - Flags: review?(terrence)
Comment on attachment 8770950 [details] [diff] [review]
improve-tenuring

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

Oh, that's much nicer! Thanks!
Attachment #8770950 - Flags: review?(terrence) → review+
The changes to the unique ID sweeping caused a test failure with compacting GC.  I'm not sure what's going on yet but I'm going to land the rest of the changes anyway.
Keywords: leave-open
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5f295fe7c321
Refactor tracing the whole cell store buffer to hopefully improve tenuring performance r=terrence
It seems updating the unique ID tables during sweeping is not valid.  If an attempt is made to hash a pointer to a cell that has been moved from the nursery before the table is updated, a new and different ID will be created for it.  This happens when we sweep the watchpoint map.

If we are able to get rid of all hash tables keyed on cell pointers apart from the unqiue ID one then I think this will work.
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/5f295fe7c321
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.