Closed Bug 1362956 Opened 5 years ago Closed 5 years ago
Moving array elements can be slow due to GC barriers
It looks like we fire a lot of GC barriers when moving array elements around, specifically: 1. During an incremental GC we need to invoke the pre-barrier on all overwritten elements due to the fact that we may be in the middle of incrementally marking the array. 2. The post barrier for generational GC scan the all modified elements looking for nursery pointers. This can cause jank when modifying large arrays.
I wonder whether we could alleviate this to some extent if push/pop didn't actually move anything in memory but just changed the array start/end positions. I assume we've considered that anyway to make push/pop faster, right?
(In reply to Boris Zbarsky [:bz] (still a bit busy) (if a patch has no decent message, automatic r-) from comment #1) > I wonder whether we could alleviate this to some extent if push/pop didn't > actually move anything in memory but just changed the array start/end > positions. I assume we've considered that anyway to make push/pop faster, > right? For shift() we have bug 1348772. Jon, if this is indeed happening a lot for shift calls, I can prioritize that bug...
Hmm. Maybe it was shift() that was showing up with the barriers in my profiles, not push/pop. That would make some sense.... I'll try to reprofile the next time this comes up.
(In reply to Jon Coppeard (:jonco) from comment #0) > 2. The post barrier for generational GC scan the all modified elements > looking for nursery pointers. Are we always using the elements barrier rather than the whole cell barrier for arrays? If not, I expect we could get some wins by moving more things over. I guess we must be because all the array methods are self-hosted now right?  http://searchfox.org/mozilla-central/rev/8b70b0a5038ef9472fe7c53e04a70c978bb06aed/js/src/jit/VMFunctions.cpp#681
Microbenchmark to compare shift against pop for large arrays. Length: Iterations: GC state: Contents: Shift time: Pop time: 100000 1000 no GC objects 0.14057 0.0013379 100000 1000 no GC numbers 0.11656 0.0011770 100000 1000 in GC objects 2.5354 0.0011880 100000 1000 in GC numbers 0.58563 0.0011379 From this it seems that using shift while in an incremental GC is particularly bad. The comment in NativeObject::moveDenseElements explains why we currently have to trigger the pre-barrier on every moved element, not just ones that are overwritten: http://searchfox.org/mozilla-central/source/js/src/vm/NativeObject-inl.h#175 Jan, bug 1348772 could totally solve this (the GC marking would have to subtract numShiftedElements when resuming marking elements).
(In reply to Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8] from comment #4) The problem here seems to be the pre-barrier for incremental GC rather than the post-barrier. I can't find where the JIT triggers a post barrier for this, but certainly we use the slots/elements store buffer for this in the interpreter (see NativeObject::elementsRangeWriteBarrierPost).
See the results in bug 1348772 comment 4, it should fix the shift() case.
Fixed by the changes in bug 1348772.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1348772
You need to log in before you can comment on or make changes to this bug.