Last Comment Bug 743854 - GC: extra barriers in ArrayBuffer::create
: GC: extra barriers in ArrayBuffer::create
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla15
Assigned To: Terrence Cole [:terrence]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-09 16:03 PDT by Terrence Cole [:terrence]
Modified: 2012-05-16 20:00 PDT (History)
2 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v0 (1.56 KB, patch)
2012-04-09 17:07 PDT, Terrence Cole [:terrence]
no flags Details | Diff | Splinter Review
v1: Keep the Class in a shared local. (1.42 KB, patch)
2012-05-16 10:58 PDT, Terrence Cole [:terrence]
wmccloskey: review+
Details | Diff | Splinter Review

Description Terrence Cole [:terrence] 2012-04-09 16:03:56 PDT
ArrayBuffers have 16 inline slots that the typearray uses to store arbitrary data.  Out of ArrayBuffer::create we end up calling JSObject::create, which calls initializeSlotRange, which calls init() on each of the 16 inline slots.  The post barrier puts a reference to each of these in the write buffer.  The typearray code goes on to write arbitrary data to these slots.  When we GC, we attempt to figure out if these values contain a GCThing and potentially dereference the "pointers" stored there.  These contain garbage: kaboom.
Comment 1 Terrence Cole [:terrence] 2012-04-09 17:07:27 PDT
Created attachment 613440 [details] [diff] [review]
v0

Simple, but ugly.  I'm not sure if it would be worth adding a special class flag for only this one case.
Comment 2 Bill McCloskey (:billm) 2012-04-19 15:16:11 PDT
Is there any way we can do this without loading the class from the shape? Most of the callers I see already have the class in a local variable.
Comment 3 Terrence Cole [:terrence] 2012-05-15 18:01:56 PDT
This code already calls shape->getObjectClass()->getPrivate() in the branch immediately above this one.  We should just store that and re-use it.
Comment 4 Terrence Cole [:terrence] 2012-05-16 10:58:17 PDT
Created attachment 624446 [details] [diff] [review]
v1: Keep the Class in a shared local.

Bill, I talked this over with Waldo and sfink this morning. Slotspan is just used differently by ArrayBuffers, so it seems that doing this specialization is the right way to go for now.
Comment 5 Bill McCloskey (:billm) 2012-05-16 11:01:49 PDT
Comment on attachment 624446 [details] [diff] [review]
v1: Keep the Class in a shared local.

OK, but please call it clasp.
Comment 6 Terrence Cole [:terrence] 2012-05-16 12:03:40 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8f13db7bff9
Comment 7 Ryan VanderMeulen [:RyanVM] 2012-05-16 20:00:21 PDT
https://hg.mozilla.org/mozilla-central/rev/a8f13db7bff9

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