Fix hardcoded quantities in nursery string pretenuring decision
Categories
(Core :: JavaScript: GC, enhancement, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox84 | --- | fixed |
People
(Reporter: sfink, Assigned: allstars.chh)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
Right now, we start pretenuring strings if you tenure 30000 of them during a minor GC. This value should really be based on the size of the nursery. (Arguably, it could also consider how much of the nursery you consumed at the time of the minor GC, but that runs the risk of pretenuring on tiny minor GCs that were triggered for a reason other than the nursery filling up.)
Updated•6 years ago
|
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 1•4 years ago
|
||
Assignee | ||
Comment 2•4 years ago
|
||
Hi Jonco, in my WIP patch I add an uint32_t called 'stringsAllocated' in Zone, and I try to increment that variable in GCRuntime::tryNewNurseryString and MacroAssembler::nurseryAllocateString. But the TSAN will report data race in MacroAssembler::nurseryAllocateString as that function will be executed both on main thread and helper thread (ion compile). Could you suggest some better way to do this in MacroAssembler?
Thanks
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 3•4 years ago
|
||
Hi Jonco
This patch does the increment in assembly so the TSAN data race problem is gone.
One question I have is how to determine the threshold? So far I've set it to 0.9, and in JetSteam benchmark I found that when the tenured strings > 30000 (the original threshold), however the tenured rate is like 0.55.
Also the Warp bug you mentioned earlier should be bug 1629778.
Comment 4•4 years ago
|
||
(In reply to Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh] from comment #3)
Great.
That's a good question. 0.9 is probably too high, but I don't know what the best value is.
You could do some benchmarking and see what effect it has on the result.
Comment 6•4 years ago
|
||
bugherder |
Description
•