Last Comment Bug 743854 - GC: extra barriers in ArrayBuffer::create
: GC: extra barriers in ArrayBuffer::create
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
-- normal (vote)
: mozilla15
Assigned To: Terrence Cole [:terrence]
: Jason Orendorff [:jorendorff]
Depends on:
  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:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

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 User image 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 User image Terrence Cole [:terrence] 2012-04-09 17:07:27 PDT
Created attachment 613440 [details] [diff] [review]

Simple, but ugly.  I'm not sure if it would be worth adding a special class flag for only this one case.
Comment 2 User image 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 User image 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 User image 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 User image 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 User image Terrence Cole [:terrence] 2012-05-16 12:03:40 PDT
Comment 7 User image Ryan VanderMeulen [:RyanVM] 2012-05-16 20:00:21 PDT

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