1060053 - Store data pointer consistently between ArrayBufferObject and SharedArrayBuffer

Status: RESOLVED INVALID

(Core :: JavaScript Engine, defect)

Description

[Brian Hackett [Laid off!]]

Attachment: patch

Currently, array buffer objects store their data pointer in one of their slots, while shared array buffers put a null pointer in that slot and fetch the data pointer from their raw buffer, which is stored in a different slot.  For bug 1058340, fetching the data pointer of an array buffer needs to be fast, and storing the data pointer consistently across the types of array buffers is both faster and simpler than what is going on now.  Once created the buffer for a shared array buffer doesn't change, so this change is straightforward to make.

The attached patch makes this change, and does some other shared array buffer related cleanup.  Mainly, obj->is<ArrayBufferObject>() will now return true for shared array buffers, as would be expected given that SharedArrayBufferObject inherits from ArrayBufferObject.  This allows removal of a lot of obfuscatory code for casting objects into array buffer objects.

This cleanup will be made irrelevant I think after bug 1054882, but that seems a long ways away.

Comment 1

[Lars T Hansen [:lth]]

I am currently rewriting SharedArrayBuffer completely, see bug 1054841 and dependent bugs.  This will create a uniform representation for ArrayBuffer and SharedArrayBuffer.  I have a large patch set that I hope to put up for review starting next week.  That patch set will disentangle ArrayBufferObject and SharedArrayBufferObject, which will become unrelated data types, but with a lot of shared code.

If it's possible, please leave all AB/SAB/TypedArray code alone for now; anything you do will (a) mess up the patches I have in the worst way and (b) probably be rewritten in short order and be wasted work on your part.

If you have an interest in the design of this, please see that bug and the spec documents linked from it.  Comments are much appreciated.

Comment 2

[Brian Hackett [Laid off!]]

Comment on attachment 8480850 [details] [diff] [review]
patch

The patches Lars posted to bug 1054882 fix this issue.