Last Comment Bug 730452 - GC: make moveDenseArrayElements trigger post write barriers always
: GC: make moveDenseArrayElements trigger post write barriers always
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla14
Assigned To: Terrence Cole [:terrence]
:
:
Mentors:
Depends on:
Blocks: 673454
  Show dependency treegraph
 
Reported: 2012-02-24 14:36 PST by Terrence Cole [:terrence]
Modified: 2012-04-06 11:47 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v0 (2.11 KB, patch)
2012-04-05 17:11 PDT, Terrence Cole [:terrence]
wmccloskey: review+
Details | Diff | Splinter Review

Description Terrence Cole [:terrence] 2012-02-24 14:36:46 PST
Currently, we check compartment()->needsBarrier() before doing HeapSlot::set to move values and use memmove in most situations.  This will not trigger post write barriers in most cases.

Bill, I thought I had already fixed up this method when I was running my generational verifier: is there some benchmark where not doing the memmove is slow?  If so, I think it will be okay to replace the memmove with .init calls so that we only trigger the post barriers, since we've already ensured that pre barriers are not needed.
Comment 1 Bill McCloskey (:billm) 2012-02-24 15:00:27 PST
Sorry. I changed this because just doing the set calls was causing a big regression on Dromaeo. Anything is fine with me as long as it doesn't regress the benchmarks.
Comment 2 Terrence Cole [:terrence] 2012-02-24 15:01:52 PST
Ah, thanks!  I'll try not to make it too ugly.
Comment 3 Terrence Cole [:terrence] 2012-02-27 14:51:03 PST
A for loop that copies elements 1-by-1 is about half the performance of gcc's memmove.  This is not acceptable.  What we need to do here instead is have a write buffer for whole-object invalidation.
Comment 4 Terrence Cole [:terrence] 2012-04-05 17:11:31 PDT
Created attachment 612749 [details] [diff] [review]
v0

This is a bit silly, but it will serve to isolate the write buffer from the object internals and to make sure it doesn't get lost in rebasing if Waldo's refactoring touches moveDenseArrayElements.
Comment 5 Bill McCloskey (:billm) 2012-04-05 17:25:43 PDT
Comment on attachment 612749 [details] [diff] [review]
v0

Please add to the comment saying that for now it's just a placeholder. Otherwise someone might delete it.
Comment 6 Terrence Cole [:terrence] 2012-04-05 17:58:53 PDT
Good idea.  Although it's now occurred to me that this is true of the entire post-barrier infrastructure at the moment.  :-)

https://hg.mozilla.org/integration/mozilla-inbound/rev/e8fb716946e0
Comment 7 Matt Brubeck (:mbrubeck) 2012-04-06 11:47:32 PDT
https://hg.mozilla.org/mozilla-central/rev/e8fb716946e0

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