532774 - [webgl] implement WebGL arrays as native JS classes

Status: RESOLVED FIXED

(Core :: js-ctypes, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

WebGL defines some new types that are intended to bridge data between JS and GL; they are essentially fixed-size native-typed arrays, with a few wrinkles (the ability to have multiple arrays of different types reference the same underlying storage buffer).

See http://blog.vlad1.com/2009/11/06/canvasarraybuffer-and-canvasarray/ for an overview of how these types work.

We should implement these natively in JS, for maximum performance and traceability.

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: first attempt

Here's a first attempt at this.  I'm writing some tests now, but I wanted to throw this up to get some early feedback as I'm sure there's lots of things that I'm doing wrong.

No attempt is made here to optimize tracing this, yet.

Comment 2

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: updated patch

Less broken patch, and some tests.

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: updated, v3

One more update -- implemented slice()  (which has totally different arg semanticsthan Array.slice() -- working on fixing this in the spec), added support for 0-length arrays, and fixed some bugs.

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 416228 [details] [diff] [review]
updated, v3

>+JSBool
>+JSWebGLArrayBuffer::class_get_byteLength(JSContext *cx, JSObject *obj, jsval id, jsval *vp)
>+{
>+    JSWebGLArrayBuffer *nobj = JSWebGLArrayBuffer::FromJSObject(obj);
>+    if (!nobj)
>+        return JS_FALSE;

0. Nit: method names use camelCaps.

1. Use false and true in new code, they coerce to JSBool.

2. Fslse return from a getter (or almost any JS entry point, internal or public, taking leading cx param) means the getter failed, i.e., either an error was reported or an exception thrown, yet you didn't pass cx to FromJSObject. So you need a report here, or if passing cx lets FromJSObject report a better error, and/or consolidates the reporting code among several call paths (other calls reach FromJSObject and need to propagate failure), then do that.

>+
>+    *vp = INT_TO_JSVAL((jsint) nobj->ByteLength());

C++ call-style cast preferred in new code.

>+void
>+JSWebGLArrayBuffer::class_finalize(JSContext *cx, JSObject *obj)
>+{
>+    JSWebGLArrayBuffer *nobj = JSWebGLArrayBuffer::FromJSObject(obj);
>+    if (!nobj)
>+        return;

Here you don't want a report, and come to think of it, is FromJSObject just doing a cast on obj->getPrivate()? If so then there should be no case in which a null is returned except for the unconstructed prototype -- and that one can cause no-ops, not failures, so the getter above should return true early, not false, if obj has a null private (i.e., obj is the class prototype).

>+
>+    delete nobj;
>+
>+    obj->setPrivate(0);

Don't do this. Finalization is idempotent and infallible so there's no need, as obj is going away immediately. The GC can do its own tomb-stoning. This is not a job to delegate to every finalize implementation, or to duplicate.

>+JSBool
>+JSWebGLArrayBuffer::class_constructor(JSContext *cx, JSObject *obj,
>+                                      uintN argc, jsval *argv, jsval *rval)
>+{
>+    if (!JS_IsConstructing(cx))
>+        return JS_FALSE;

This is a silent failure, very hard to debug. Don't do it, do report an error if the spec says that calling the constructor instead of new'ing it should fail.

Most JS builtin constructors construct if called, some coerce their argument (String, Number, Boolean, some cases of Object).

>+    if (argc == 0) {
>+        // XXX not an ideal message here (entirely wrong, bad args!)
>+        JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
>+                             JSMSG_BAD_ARRAY_LENGTH);
>+
>+        return JS_FALSE;
>+    }

What does the spec say?

>+    JSWebGLArrayBuffer *nobj = new JSWebGLArrayBuffer();

Nit: nobj is a semi-canonical name for JSObject * type local -- how about glab, ab, array, arraybuf or something like that? Canonical local names help read- and grep-ability. It seems nobj is new canon here, which is good, but for the collision with JSObject *nobj elsewhere (jsarray.cpp, jsapi.cpp).

>+    if (!nobj->Allocate(len)) {

Nit: camelCaps for method names.

>+inline JSWebGLArrayBuffer *
>+JSWebGLArrayBuffer::FromJSObject(JSObject *obj) {

Left brace on own line.

Move this up above all uses.

>+JSBool
>+JSWebGLArrayBuffer::Allocate(uint32 byteLength) {

Left brace nit -- last mention.

>+    JS_ASSERT(mData == 0);
>+
>+    if (byteLength) {
>+        mData = calloc(byteLength, 1);
>+        if (!mData)
>+            return JS_FALSE;
>+    } else {
>+        mData = 0;
>+    }
>+
>+    mByteLength = byteLength;
>+    return JS_TRUE;

This wants true/false, cx parameter, and cx->calloc (which takes just nbytes as its param).

>+void
>+JSWebGLArray::class_finalize(JSContext *cx, JSObject *obj) {
>+    // I don't think we have anything to do here, do we?
>+}

So use JS_FinalizeStub.

>+void
>+JSWebGLArray::class_trace(JSTracer *trc, JSObject *obj)

The "class_" prefix is confusing, since this is a JSObjectOp hook (and JSClass has its own trace, neé mark, hook). Suggest objectop_ or no prefix.

>+{
>+    JSWebGLArray *wa = FromJSObject(obj);

Canonical name wa -- cool.

>+    jsval bval = OBJECT_TO_JSVAL(wa->mBufferJS);

But no null wa test -- if the prototype is unconstructed then how do you survive a null deref here?

I haven't studied the whole file yet (or the hidden spec :-/) but it seems heavy to have an object, whose private is a C++ struct or class instance owning another object. Is this required by the API?

>+    JS_CallTracer(trc, JSVAL_TO_TRACEABLE(bval), JSVAL_TRACE_KIND(bval));

No need to go through jsval bval and back, all you need is wa->mBufferJS and JSTRACE_OBJECT for the 2nd and 3rd args here.

>+JSWebGLArray::class_getAttributes(JSContext *cx, JSObject *obj, jsid id, JSProperty *prop,
>+                                  uintN *attrsp)
>+{
>+    *attrsp = id == ATOM_TO_JSID(cx->runtime->atomState.lengthAtom)
>+        ? JSPROP_PERMANENT | JSPROP_READONLY : JSPROP_ENUMERATE;

Nit: prevailing style parenthesizes non-primary-expression ?: conditions, and indents ? and : on their own lines to start in the same column as the condition on overflow.

>+JSWebGLArray::class_setAttributes(JSContext *cx, JSObject *obj, jsid id, JSProperty *prop,
>+                                  uintN *attrsp)
>+{
>+    return JS_FALSE;

Silent failure, need to report here to avoid the find-the-silent-failure-needle-in-haystack problem later.

>+inline JSBool

Use bool for private helpers.

>+JSWebGLArray::IsArrayIndex(JSContext *cx, JSObject *obj, jsid id) {
>+    JSWebGLArray *wa = FromJSObject(obj);
>+    jsuint index;
>+    return js_IdIsIndex(id, &index) &&
>+        index < wa->mLength;

Nit: indent overflowing && and || operands to start in same column as first operand.

+static JSBool
>+FastValueToInt32(JSContext *cx, jsval v, int32 *val) {

New nit: canonical name is ip, not val, for int23 * out param.

>+    if (JSVAL_IS_INT(v)) {
>+        *val = JSVAL_TO_INT(v);
>+    } else {
>+        jsdouble d;
>+        if (!JS_ValueToNumber(cx, v, &d))

Note leading cx param, meaning if this API fails, error was reported or exception thrown. Many times later, you call FastValueToInt32 and then double-report when it returns false.

Inside the JS engine sources, you should avoid calling JS_* API entry points if there are library-internal functions they call that you can call directly. However:

This helper is redundant, though. See js_ValueToInt32 in jsnum.cpp. It handles NaN and out of range (including the infinities) values, unlike the rogue cast below:

>+            return JS_FALSE;
>+        *val = (int32) d;
>+    }
>+    return JS_TRUE;
>+}

Since FastValueToInt32 is not inline I don't see why you can't replace it with fallible js_ValueToInt32. Note the optimized failure-reporting protocol there, though: instead of a bool or JSBool return type, it returns int32 directly and JSVAL_NULL in *vp on failure (it needs its jsval param to be rooted, so vp must point to a rooted jsval location).

>+template<typename T>
>+class JSWebGLTypedArray
>+    : public JSWebGLArray
>+{
>+public:
>+    static JSClass slowClass;
>+
>+    static JSClass fastClass;
>+    static JSObjectOps fastObjectOps;
>+    static JSObjectMap fastObjectMap;
>+
>+    static JSFunctionSpec jsfuncs[];
>+
>+    static JSObjectOps *getObjectOps(JSContext *cx, JSClass *clasp)
>+    {
>+        return &fastObjectOps;
>+    }
>+
>+    // This code is valid for int8, uint8, int16, uint16, but we
>+    // have custom specializations below for int32, uint32, and float
>+    static JSBool
>+    class_getProperty(JSContext *cx, JSObject *obj, jsval id, jsval *vp)

This is well-prefixed if it's the JSClass hook, not the JSObjectOp hook which takes jsid id, not jsval id (but the two types are compatible). Which is it, though?

The comment suggests that the JSObjectOps hooks are generic rather than uint8, uint16, etc. element type specific, and do not need to be replicated by template instantiation -- true?

>+    {
>+        JSWebGLTypedArray<T> *wa = JSWebGLTypedArray<T>::FromJSObject(obj);
>+
>+        if (id == ATOM_TO_JSID(cx->runtime->atomState.lengthAtom)) {
>+            *vp = INT_TO_JSVAL(wa->mLength);
>+            return JS_TRUE;
>+        }

This is good if class_getProperty is the JSObjectOps hook, not the JSClass hook. You don't want a JSClass.getProperty impl if you are implementing object-ops, so I'm assuming class_getProperty is the JSObjectOp hook -- in which case its id parameter should be typed jsid, not jsval.

>+        if (id == ATOM_TO_JSID(cx->runtime->atomState.protoAtom)) {
>+            *vp = OBJECT_TO_JSVAL(obj->getProto());
>+            return JS_TRUE;
>+        }

This is gravy, not sure __proto__ is required by ok if you like.

>+        jsuint index;
>+        if (!js_IdIsIndex(ID_TO_VALUE(id), &index)) {

Note js_IdIsIndex treats [0,uint32(0xffffffff)] as indexes, which is more than fits in an INT jsval. It seems to me that here is where you must reject out of range indexes, rather than try to fetch their values from the dense machine int typed data vector, or else chase them up the proto chain.

Don't you want IsArrayIndex?

>+            JSObject *obj2;
>+            JSProperty *prop;
>+            JSScopeProperty *sprop;
>+
>+            JSObject *proto = STOBJ_GET_PROTO(obj);
>+            if (!proto) {
>+                *vp = JSVAL_VOID;
>+                return JS_TRUE;
>+            }
>+
>+            *vp = JSVAL_VOID;
>+            if (js_LookupPropertyWithFlags(cx, proto, id, cx->resolveFlags,
>+                                           &obj2, &prop) < 0)
>+                return JS_FALSE;
>+
>+            if (prop) {
>+                if (OBJ_IS_NATIVE(obj2)) {
>+                    sprop = (JSScopeProperty *) prop;
>+                    if (!js_NativeGet(cx, obj, obj2, sprop, JSGET_METHOD_BARRIER, vp))
>+                        return JS_FALSE;
>+                }
>+                obj2->dropProperty(cx, prop);
>+            }
>+            return JS_TRUE;
>+        }

Can't you just delegate to proto->getProperty here? The lookup-then-native-get code above does nothing on foreign objects.

>+    // .. but we don't do anything custom for the setter, because the
>+    // code looks identical for all the array types (since they're
>+    // native destinatinons, and we have no knowledge of what type the
>+    // input vp is)
>+    static JSBool
>+    class_setProperty(JSContext *cx, JSObject *obj, jsval id, jsval *vp)

jsid id

>+    {
>+        JSWebGLTypedArray<T> *wa = JSWebGLTypedArray<T>::FromJSObject(obj);
>+
>+        if (id == ATOM_TO_JSID(cx->runtime->atomState.lengthAtom)) {
>+            *vp = INT_TO_JSVAL(wa->mLength);
>+            return JS_TRUE;
>+        }

No __proto__ handling here -- trouble since it is settable but delegating to the proto will set the grand-proto!

>+
>+        jsuint index;
>+        // we can't just chain to js_SetProperty, because we're not a normal object
>+        if (!js_IdIsIndex(ID_TO_VALUE(id), &index))

See above.

>+            return JS_FALSE;

Silent failure! You do not want to no-op non-index assignments, rather throw an exception by reporting an error.

>+
>+        if (index >= wa->mLength)
>+            return JS_FALSE;

Silent failure again -- don't do it!

>+    class_enumerate(JSContext *cx, JSObject *obj, JSIterateOp enum_op,
>+                    jsval *statep, jsid *idp)
>+    {
>+        JSWebGLTypedArray<T> *wa = JSWebGLTypedArray<T>::FromJSObject(obj);
>+
>+        switch (enum_op) {
>+        case JSENUMERATE_INIT:

Nit: half-indent (two-space offset) case and goto labels per house style.

>+            *statep = JSVAL_ZERO;
>+            if (idp)
>+                *idp = INT_TO_JSID(wa->mLength);
>+            break;
>+
>+        case JSENUMERATE_NEXT: {
>+            int32 curVal = JSVAL_TO_INT(*statep);
>+
>+            *idp = INT_TO_JSID(curVal);
>+
>+            if (curVal == wa->mLength) {
>+                *statep = JSVAL_NULL;
>+            } else {
>+                *statep = INT_TO_JSVAL(curVal+1);
>+            }

Nit: no need to over-brace, indeed better to set *statep = (...) ? ... : ... here.

>+        }
>+            break;

break inside braces (closing brace half-indented to match case label start).

>+
>+        case JSENUMERATE_DESTROY:
>+            break;

Need to set *statep = JSVAL_NULL here, right?

>+    /*
>+     * new JSWebGL[Type]Array(length)
>+     * new JSWebGL[Type]Array(otherWebGLArray)
>+     * new JSWebGL[Type]Array(JSArray)
>+     * new JSWebGL[Type]Array(WebGLArrayBuffer, [optional] byteOffset, [optional] length)
>+     */
>+    static JSBool
>+    class_constructor(JSContext *cx, JSObject *obj,
>+                      uintN argc, jsval *argv, jsval *rval)
>+    {
>+        //
>+        // Note: this is a constructor for slowClass, not fastClass!
>+        //
>+
>+        if (!JS_IsConstructing(cx))
>+            return JS_FALSE;

No silent failures! But what does the spec say, again? An error on a call without new is user-hostile and pedantic for no gain in code size or usability.

>+    /* slice(offset, length) */

The spec had better change -- it's nuts to ignore Python and JS (ECMA-262 Edition 3) precedent and call something slice that does not take [begin, end) params, including the negative-means-from-end behavior.

Out of time now, hope you can revise globally based on above. Failure handling in particular needs close attention and fixage. Minimizing the duplication of code by template instantiation is worth doing if possible.

/be

Comment 5

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: updated, v4

Thanks for the review!  Here's an updated patch that addresses most of
the review comments, except for the getProperty delegation -- question
below about that.

(In reply to comment #4)
> (From update of attachment 416228 [details] [diff] [review])
> >+JSBool
> >+JSWebGLArrayBuffer::class_get_byteLength(JSContext *cx, JSObject *obj, jsval id, jsval *vp)
> >+{
> >+    JSWebGLArrayBuffer *nobj = JSWebGLArrayBuffer::FromJSObject(obj);
> >+    if (!nobj)
> >+        return JS_FALSE;
> 
> 0. Nit: method names use camelCaps.

Fixed.  I've changed the methods to be class_ for things that are in
JSClass, obj_ for JSObjectOp, prop_ for functions used in
JSPropertySpec tables, and fun_ for JSFunctionSpec functions.
Everything after the _ is camelcased, e.g. prop_getByteLength.  Does
that work?  I can move to propGetByteLength too, but this way seemed
clearer.

> 1. Use false and true in new code, they coerce to JSBool.

Done.

> 2. Fslse return from a getter (or almost any JS entry point, internal or
> public, taking leading cx param) means the getter failed, i.e., either an error
> was reported or an exception thrown, yet you didn't pass cx to FromJSObject. So
> you need a report here, or if passing cx lets FromJSObject report a better
> error, and/or consolidates the reporting code among several call paths (other
> calls reach FromJSObject and need to propagate failure), then do
> that.

Fixed a bunch of error reporting and the like; there should be no
silent-false returns any more.  In a few places I found that the shell
would just die if I did do a silent-false return, that seems weird.

> Most JS builtin constructors construct if called, some coerce their argument
> (String, Number, Boolean, some cases of Object).

I just changed them to construct if called.  It's a little odd, as the
spec doesn't talk about this -- the spec's written using WebIDL, so we
should do whatever WebIDL specifies for [[constructor]] methods.


> >+    if (argc == 0) {
> >+        // XXX not an ideal message here (entirely wrong, bad args!)
> >+        JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
> >+                             JSMSG_BAD_ARRAY_LENGTH);
> >+
> >+        return JS_FALSE;
> >+    }
> 
> What does the spec say?

Changed the error message, but it's still an error.  It's not really
useful to have this as a shorthand for constructing a 0-length buffer
or array, because you can't ever expand it like you can with a normal Array.

> >+void
> >+JSWebGLArray::class_finalize(JSContext *cx, JSObject *obj) {
> >+    // I don't think we have anything to do here, do we?
> >+}
> 
> So use JS_FinalizeStub.

Or, you know, delete the inner C++ object.  Oops, fixed.  :-)

> >+{
> >+    JSWebGLArray *wa = FromJSObject(obj);
> 
> Canonical name wa -- cool.

Switched everything to using  'wb' - WebGLArrayBuffer, 'wa' -
WebGLArray/WebGLTypedArray, and 'nwb'/'nwa' variatns for new instances
of those.


> >+    jsval bval = OBJECT_TO_JSVAL(wa->mBufferJS);
> 
> But no null wa test -- if the prototype is unconstructed then how do you
> survive a null deref here?

Fixed.

> I haven't studied the whole file yet (or the hidden spec :-/) but it seems
> heavy to have an object, whose private is a C++ struct or class instance owning
> another object. Is this required by the API?

The ArrayBuffers are just chunks of memory, and the Arrays are views
on top of the ArrayBuffer; you can have multiple overlapping views all
referencing the same portions of an ArrayBuffer.  I /could/ get rid of
the inner C++ classes here and just store all my data in reserved
slots on the JSObject; I did it this way because I'd like C++ code to
be able to get the C++ class (via fromJSObject) and interact with it
instead of having to deal with the JSObject, possibly teaching
XPConnect about these classes and how to convert.  The latter part
would involve creating a wrapper class each time through xpconnect, I
think, and I wanted to avoid that.

> Nit: indent overflowing && and || operands to start in same column as first
> operand.
> 
> +static JSBool
> >+FastValueToInt32(JSContext *cx, jsval v, int32 *val) {
> [...] 
> This helper is redundant, though. See js_ValueToInt32 in jsnum.cpp. It handles
> NaN and out of range (including the infinities) values, unlike the rogue cast
> below:

Yeah, I got rid of it.  It was an overoptimization that I pulled out
as an inline function and then it just got out of hand from there, and
turned into a poor copy of js_ValueToInt32.

> >+    // This code is valid for int8, uint8, int16, uint16, but we
> >+    // have custom specializations below for int32, uint32, and float
> >+    static JSBool
> >+    class_getProperty(JSContext *cx, JSObject *obj, jsval id, jsval *vp)
> 
> This is well-prefixed if it's the JSClass hook, not the JSObjectOp hook which
> takes jsid id, not jsval id (but the two types are compatible). Which is it,
> though?

Fixed.

> The comment suggests that the JSObjectOps hooks are generic rather than uint8,
> uint16, etc. element type specific, and do not need to be replicated by
> template instantiation -- true?

Oops, comments were leftovers from an earlier implementation.  But
yes, the getProp/setProp hooks are generic; they internally call
copyIndexToValue and setIndex which are template-specialized for the
specific type.

> >+        if (id == ATOM_TO_JSID(cx->runtime->atomState.protoAtom)) {
> >+            *vp = OBJECT_TO_JSVAL(obj->getProto());
> >+            return JS_TRUE;
> >+        }
> 
> This is gravy, not sure __proto__ is required by ok if you like.

This was copy-and-paste; nuked this.

> >+        jsuint index;
> >+        if (!js_IdIsIndex(ID_TO_VALUE(id), &index)) {
> 
> Note js_IdIsIndex treats [0,uint32(0xffffffff)] as indexes, which is more than
> fits in an INT jsval. It seems to me that here is where you must reject out of
> range indexes, rather than try to fetch their values from the dense machine int
> typed data vector, or else chase them up the proto chain.
> 
> Don't you want IsArrayIndex?

Yep, I sure do.  Made isArrayIndex take a cx and optionally return the
jsuint index, doing length checks along the way.

> >+            JSObject *obj2;
> >+            JSProperty *prop;
> >+            JSScopeProperty *sprop;
> >+
> >+            JSObject *proto = STOBJ_GET_PROTO(obj);
> >+            if (!proto) {
> >+                *vp = JSVAL_VOID;
> >+                return JS_TRUE;
> >+            }
> >+
> >+            *vp = JSVAL_VOID;
> >+            if (js_LookupPropertyWithFlags(cx, proto, id, cx->resolveFlags,
> >+                                           &obj2, &prop) < 0)
> >+                return JS_FALSE;
> >+
> >+            if (prop) {
> >+                if (OBJ_IS_NATIVE(obj2)) {
> >+                    sprop = (JSScopeProperty *) prop;
> >+                    if (!js_NativeGet(cx, obj, obj2, sprop, JSGET_METHOD_BARRIER, vp))
> >+                        return JS_FALSE;
> >+                }
> >+                obj2->dropProperty(cx, prop);
> >+            }
> >+            return JS_TRUE;
> >+        }
> 
> Can't you just delegate to proto->getProperty here? The lookup-then-native-get
> code above does nothing on foreign objects.

This was copy and paste again; do I just return proto->getProperty(cx,
id, id) ?

> >+    /* slice(offset, length) */
> 
> The spec had better change -- it's nuts to ignore Python and JS (ECMA-262
> Edition 3) precedent and call something slice that does not take [begin, end)
> params, including the negative-means-from-end behavior.

Yep, got the change through.  I've updated the code to use begin,end
and otherwise have same semantics as Array.slice.

Comment 6

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: updated, v5

Updated, now with some terrifying js standard class stuffs so that it shows up in xpconnect contexts (which don't use InitStandardClasses).

Comment 7

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: updated, v6

Updated again.  Not a lot of changes from v5, but added the WebGL types to the standard classes (as this is what xpconnect initializes in new contexts).

Comment 8

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Naming... these will have to get aliased to the WebGL* names, but they don't have to be named that way as the base name.  Brendan suggested {Float,Int,Uint}{8,16,32}{Array,Buffer} which almost works, except that there's only one "Buffer", and it doesn't have a type associated with it.  (The buffer is the actual data container, the arrays are merely accessors for that container.  The buffer might be created implicitly by constructing a typed array.)

I suggest a prefix; maybe one of 'Typed', 'Raw', or 'Binary':

TypedArrayBuffer     RawArrayBuffer     BinaryArrayBuffer
TypedUint16Array     RawUint16Array     BinaryUint16Array
TypedFloat32Array    RawFloat32Array    BinaryFloat32Array

maybe "Fixed", since the size of all these things is fixed at creation:

FixedArrayBuffer
FixedUint16Array
FixedFloat32Array

.. though that has some confusion with fixed-point numeric representation.  'Native' is also possible, though it's somewhat overused already.

Comment 9

[Brendan Eich [:brendan]]

ArrayBuffer
{Float}{32,64}Array
{Int,Uint}{8,16,32}Array

Base class: TypedArray in namespace js (C++ namespace instead of JSFoo)

Filename: jstypedarray.cpp

/be

Comment 10

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: updated, v7

Updated; uses new type names.

Comment 11

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

One small change in my local tree: instead of JS_CallTracer, it uses JS_CALL_OBJECT_TRACER.

Comment 12

[Brendan Eich [:brendan]]

s/INVALID_/BAD_/ and columnate for best js.msg effect

s/\<wb\>/ab/g for new canonical name, no WebGL vestiges.

In ArrayBuffer::prop_getByteLength, use just one return true and test if (wb)
to guard the store through *vp, for best prefetched-is-predicted execution
order, no PGO required.

Same comment for ArrayBuffer::class_finalize and any others like it -- we do
not prefer early return if the resulting over-indentation is minor, provided
the prefetched-is-predicted win is clear.

Nit: could fit

    if (!JSVAL_IS_INT(argv[0]) ||
        (len = JSVAL_TO_INT(argv[0])) < 0)
    {

all on one line. And in the "then" block for that if, kill the blank line
before the return false.

Test for null wb (we will get to a better OOM policy but need the Electrolysis
work to monitor headroom):

    ArrayBuffer *wb = new ArrayBuffer();

    if (!wb->allocateStorage(cx, len)) {

Don't double-report OOM, though -- wb->allocateStorage should have done the
report already since it is a fallible method taking a leading cx. You'll want

        if (wb) {
            delete wb;
        } else {
            JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
                                 JSMSG_OUT_OF_MEMORY);
        }

Nit (last time): can lose blank line between return and preceding statement:

    obj->setPrivate(wb);

    return true;

Per above, ArrayBuffer::allocateStorage() must call JS_ReportOutOfMemory if
cx->calloc fails.

s/wa/ta/g for new canonical TypedArray* name.

Don't TypedArray::prop_getBuffer etc. need to null test the result of their
fromJSObject calls, to handle the unconstructed prototype (which is script-
addressable)? If JSCLASS_CONSTRUCT_PROTOTYPE helps, use it, but beware it
does call the constructor with zero args.

Nit: we don't use mLength, etc., rather just length -- any ambiguities, use
this->length.

Nit: parenthesize the ?: condition in TypedArray::obj_getAttributes.

Question about TypedArray::obj_getAttributes: aren't the indexed properties
permanent as well as enumerable?

Nit in TypedArray::isArrayIndex: the if condition and brace can go on one line.

Does it hurt to put this inline helper above all its uses? It hurts my brain
slightly not to ;-).

Nit: half-indent the superclass declaration in

class TypedArrayTemplate
    : public TypedArray

In class TypedArrayTemplate::obj_getProperty, invert this if condition:

        if (!wa->isArrayIndex(cx, id, &index)) {

to make the indexed-get case the prefetched-is-predicted-taken fast path.

Potential nit once you do that and reindent:

            if (js_LookupPropertyWithFlags(cx, proto, id, cx->resolveFlags,
                                           &obj2, &prop) < 0)
                return false;

This needs bracing, on account of multiline condition (any of condition, "then"
or "else" clause being multiline triggers bracing, in house style). But we have
tw=99 in new code, so you really don't need to wrap the long call, especially
once you have unindented.

In TypedArrayTemplate::obj_deleteProperty, you have

        if (wa && wa->isArrayIndex(cx, id)) {
            *rval = JSVAL_FALSE;
            return true;
        }

which means that TypedArray::obj_getAttributes should *definitely* return
JSPROP_PERMANENT along with JSPROP_ENUMERABLE.

In TypeArrayTemplate::obj_enumerate, use jsint rather than int32 curVal.

Nit: in class_constructor, use one-line (//...) or multiline (/*\n... * ... */)
style for this comment:

Regarding this comment and its code:

            // cases -- we should check to see if there's an error
            // before setting this one.

you should always report an error and return false/null ASAP, and never end up
in this conundrum.

In TypedArrayTemplate::class_finalize, one return for prefetched-is-predicated.

Re: TypedArrayTemplate::fun_slice, nice! Did the standards body agree to be
Pythonic/JS-ic?

Regarding this line in fun_slice:

            // false? true?

you don't need to test for null |this| -- the runtime will pass the right
object in any event (global for null).

            // this should rarely ever happen

Is this really an error, or just a degenerate (returns []) slice?

Is the TypedArrayTemplate(JSContext *cx, uint32 length) fallible? It tests:

        if (!createBufferWithSizeAndCount(cx, sizeof(T), length))
            return;

Looks like a suppressed error. Should be in a checked init method. Or does the
spec say otherwise?

Same for the four-arg TypedArrayTemplate.

In four-arg TypedArrayTemplate, byteOffset initial assignment should use ?:
instead of overbraced if/else, and should be an initializer for the uint32
byteOffset; declaration.

The "// invalid byteOffset" commented return needs an error report, I hope (per
spec). But why does this ctor take an argument that can be misaligned? What's
the use-case?

Related: why byteOffsetInt at all, allowing -1 default param or caller to pass
negative any-int just to force byteOffset to 0?

Nit: spaces around * in:

            mByteLength = length*sizeof(T);

If other is neither Array, not TypedArray, nor ArrayBuffer, should this ctor
just silently do nothing?

Is slice still mis-spec'ed?

XXX comments in copyFrom need resolution or FIXME: bug ...... citations.

Nit: switch case labels and closing braces half-indented in second copyFrom
overloading -- also, break inside braced block, not after and yet oddly indented
compared to the closing brace.

Nit: opening function body brace on its own line in

TypedArrayTemplate<T>::copyIndexToValue(JSContext *cx, uint32 index, jsval *vp) {

and others.

Nit: \ in column 79 per house style in

#define IMPL_TYPED_ARRAY_STATICS(_typedArray)                           \

and wrap overlong lines as usual (after fastObjectMap's return type, formal
param, etc.).

Nit: as with case and goto labels, house style generally half-indents private:,
protected:, and public:.

Nits, silent failures, missing errors, and error-begging param types aside,
this code is hawt ;-).

/be

Comment 13

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Got most of these, some questions:

(In reply to comment #12)
> s/\<wb\>/ab/g for new canonical name, no WebGL vestiges.

Used 'abuf' and 'tarray'; 'ab' and 'ta' were getting a little too shorthand.

> Don't TypedArray::prop_getBuffer etc. need to null test the result of their
> fromJSObject calls, to handle the unconstructed prototype (which is script-
> addressable)? If JSCLASS_CONSTRUCT_PROTOTYPE helps, use it, but beware it
> does call the constructor with zero args.

Hmm... I originally changed the constructors to just assume length=0 if called with no args.  The spec might take issue with this, since arguably a length=0 array is not all that useful, but length=0 is explicitly allowed.  But this ended up being tricky, since I had to prevent my slow->fast morphing here, so just gave up and added null checks in the various property getters.

> Nit: we don't use mLength, etc., rather just length -- any ambiguities, use
> this->length.

Hmm, what do I do with my read-only accessors for these, that are called length() now?  uint32 length; uint32 getLength() ?  Or do I just get rid of the protected bit and just assume people will know what they're doing with an admittedly private/friend API? 

In a constructor, where I want an int arg, I switched to just using JS_ValueToECMAInt32, but that seems to happily convert "abc" to 0.  Seems like that should throw?  I guess I'm getting "abc" -> NaN -> 0, but that seems like an unfortunate sequence.

> Regarding this comment and its code:
> 
>             // cases -- we should check to see if there's an error
>             // before setting this one.
> 
> you should always report an error and return false/null ASAP, and never end up
> in this conundrum.

Fixed; I had to move to init() methods to simplify this, because there was no way to indicate whether the constructor had set an error or not.

> Re: TypedArrayTemplate::fun_slice, nice! Did the standards body agree to be
> Pythonic/JS-ic?

Yeah; I don't know if it's reflected in the spec yet, I think it might be on me to change the language there.

>             // this should rarely ever happen
> Is this really an error, or just a degenerate (returns []) slice?

It's really an error -- we do lots of error checking to make sure we pass valid args, but if that function returns NULL, something very bad is happening, though potentially (most commonly) OOM.

> The "// invalid byteOffset" commented return needs an error report, I hope (per
> spec). But why does this ctor take an argument that can be misaligned? What's
> the use-case?
> 
> Related: why byteOffsetInt at all, allowing -1 default param or caller to pass
> negative any-int just to force byteOffset to 0?

The constructor has these as optional args, defaulting to sane values.  Unfortunately, 0 is a valid value.  I could calculate the correct default values before calling this init function in the constructor, though.

As for why it can take unaligned args, we went back and forth on whether these values should be in bytes or in units of whatever the type is; we settled on bytes so that there was a common API to all of them, and so that people didn't have to do mental arithmetic when trying to lay out a complex sequence of types in a buffer. ("I have two shorts followed by 4 bytes and then the float that I want... so I need to 2*2+4, 8 bytes, but sizeof float is 4, so I need to pass 2 for the offset to get my float in the right place")

> If other is neither Array, not TypedArray, nor ArrayBuffer, should this ctor
> just silently do nothing?

Sure shouldn't; fixed.

> XXX comments in copyFrom need resolution or FIXME: bug ...... citations.

Yep, will file.

Will post an updated patch soon.

Comment 14

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: updated, v8

Updated; I just got rid of the protected members and read-only accessors -- if JSObject is wide-open, who am I to try to be more sneaky?

Comment 15

[Brendan Eich [:brendan]]

Comment on attachment 417282 [details] [diff] [review]
updated, v8

Nit: major comment style (<indent>/*\n<indent> *...) here:

        /* We're just not going to support arrays that are bigger than what will fit

Use nbytes as the canonical name, bytes is canonical elsewhere in spidermonkey
for const char * pointer var:

ArrayBuffer::allocateStorage(JSContext *cx, uint32 bytes)

In ArrayBuffer::freeStorage, the #ifdef DEBUG data nulling should be in the
"then" clause of the if (data).

Nit for ArrayBuffer::freeStorage and ArrayBuffer::~ArrayBuffer and possibly
others: use NULL not 0 per house style.

Nit: use C++ call-style cast in

            tarray->setIndex(index, (T) JSVAL_TO_INT(*vp));

and similar.

In TypedArrayTemplate::obj_deleteProperty, you assert that obj is not the class
prototype, if I'm reading this right:

        JS_ASSERT(tarray);

but of course you null defend elsewhere. Why not here?

In obj_deleteeProperty, don't declare or init tarray until you need it.

Is JSMSG_BAD_ARRAY_LENGTH the right error for that ctor, which takes an
argument of several types? 

Nit: in TypedArrayTemplate::init, oversharing return false; makes for ugly
multiline if conditions and extra braces, e.g.:

            if (!JS_GetArrayLength(cx, other, &len) ||
                !createBufferWithSizeAndCount(cx, sizeof(T), len) ||
                !copyFrom(cx, other, len))
            {
                return false;
            }

Better to write one-line-per-condition-and-consequent separate if-then
statements -- same number of gross lines, note well!

Nit (still :-/) -- the if-else starting here is overbraced:

            if (byteOffsetInt < 0) {

Hmm, almost all nits. One more patch and I hope to r+ (I will, with any nits to be fixed before checkin).

/be

Comment 16

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Updated patch coming, but:

> In TypedArrayTemplate::obj_deleteProperty, you assert that obj is not the class
> prototype, if I'm reading this right:
> 
>         JS_ASSERT(tarray);
> 
> but of course you null defend elsewhere. Why not here?

The obj ops are only present/called on the fast arrays, not the class prototype -- all the obj_* ops JS_ASSERT(tarray), but that should be ok, right?

Also changed the template param "T" to "NativeType" to make it clearer.

Comment 17

[Brendan Eich [:brendan]]

(In reply to comment #16)
> The obj ops are only present/called on the fast arrays, not the class prototype
> -- all the obj_* ops JS_ASSERT(tarray), but that should be ok, right?

Oh, for sure. Sorry for the false alarm. New patch before Christmas?

/be

Comment 18

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: updated, v9

Ok, think I got everything in here.

One question -- where should the tests go?  It's a custom-written quick hack harness, but I'd be happy to convert it to some standard suite... nothing in js/src/tests seemed obvious though, as it was all tied to a specific version.

Comment 19

[Brendan Eich [:brendan]]

Comment on attachment 418796 [details] [diff] [review]
updated, v9

> One question -- where should the tests go?  It's a custom-written quick hack
> harness, but I'd be happy to convert it to some standard suite... nothing in
> js/src/tests seemed obvious though, as it was all tied to a specific version.

We use js/src/trace-test for JIT tests, FWIW. Not sure what's best here. Suggest you ask bc, dmandelin, and jorendorff for their thoughts.

>+            uint32 boffset, len;
>+
>+            //printf ("buffer: %d %d %d\n", abuf->byteLength, abuf->byteLength / sizeof(NativeType), len * sizeof(NativeType) == abuf->byteLength);
>+            if (byteOffsetInt < 0)
>+                boffset = 0;
>+            else
>+                boffset = (uint32) byteOffsetInt;

Declare boffset with a ?: initializer expression to avoid this if-else.

>+
>+            if (boffset > abuf->byteLength ||
>+                boffset % sizeof(NativeType) != 0)
>+            {

Nit: fit on one line.

>+            if (lengthInt < 0) {
>+                len = (abuf->byteLength - boffset) / sizeof(NativeType);
>+                if (len * sizeof(NativeType) != (abuf->byteLength - boffset)) {
>+                    JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
>+                                         JSMSG_TYPED_ARRAY_BAD_ARGS);
>+                    return false; // given byte array doesn't map exactly to sizeof(NativeType)*N
>+                }
>+            } else {
>+                len = (uint32) lengthInt;
>+            }

Looks like len's decl hsould move down to before this if-else.

>+    slice(uint32 begin, uint32 end)
>+    {
>+        if (begin > length ||
>+            end > length)
>+        {

Nit: all on one line please.

>+        tarray->byteLength = (end-begin)*sizeof(NativeType);
>+        tarray->length = (end-begin);

Nit: spaces around binary operators.

>+            for (uintN i = 0; i < len; ++i) {
>+                if (JS_GetElement(cx, ar, i, &v)) {
>+                    if (JSVAL_IS_INT(v)) {
>+                        *dest++ = NativeType(JSVAL_TO_INT(v));
>+                    } else if (JSVAL_IS_DOUBLE(v)) {
>+                        *dest++ = NativeType(*JSVAL_TO_DOUBLE(v));
>+                    } else {
>+                        jsdouble dval;
>+                        if (!JS_ValueToNumber(cx, v, &dval))
>+                            return false;
>+                        *dest++ = NativeType(dval);
>+                    }
>+                } else {
>+                    return false;
>+                }

Nit: if (!JS_GetElement...) and return false early to avoid overindenting and making the vestigial else decried by Brian Kernighan.

>+        switch (tarray->type) {
>+        case TypedArray::TYPE_INT8: {
>+            int8 *src = static_cast<int8*>(tarray->data);
>+            for (uintN i = 0; i < length; ++i)
>+                *dest++ = NativeType(*src++);
>+            break;
>+        }

Uber-nit: prevailing style half-indents case and goto (and visibility) labels, and if braced like this, the closing brace gets two-space indented too.

>+        if (!JS_NewDoubleValue(cx, (jsdouble) val, vp))

Not so much a nit, more an easy speedup: avoid the JS API since you are inside the engine, use the jsnum.h internal API here.

Same comment applies elsewhere for other APIs that have faster internal entry points, including JS_GetElement. See the jsapi.cpp source to find the lower level entry points.

>+js_IsTypedArray(JSObject *obj)
>+{
>+    if (obj) {
>+        // Order is chosen based on guesstimate of most common class usage.
>+        return obj->getClass() == &Float32Array::fastClass ||
>+               obj->getClass() == &Uint16Array::fastClass ||
>+               obj->getClass() == &Uint8Array::fastClass ||
>+               obj->getClass() == &Int8Array::fastClass ||
>+               obj->getClass() == &Int16Array::fastClass ||
>+               obj->getClass() == &Int32Array::fastClass ||
>+               obj->getClass() == &Uint32Array::fastClass;

Hey, you have an array of pointers to these classes (see below). Why not just allocate the structs as array elements and you can range-test here, no need to order and worry about slower paths if guesstimate is wrong.

>+    enum {
>+        TYPE_INT8 = 0,
>+        TYPE_UINT8 = 1,
>+        TYPE_INT16 = 2,
>+        TYPE_UINT16 = 3,
>+        TYPE_INT32 = 4,
>+        TYPE_UINT32 = 5,
>+        TYPE_FLOAT32 = 6,
>+        TYPE_MAX

Any reason to assign explicit enumerator values? If not matching external data format, I'd let the compiler. You can use a JS_STATIC_ASSERT to make sure any array initializer lines up.

>+    static JSClass *fastClasses[TYPE_MAX];

See above comment on arraying the class structs instead of pointers.

>\ No newline at end of file

Please fix this while you are at it. I will r+ before xmas -- ho ho ho!

/be

Comment 20

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Argh.. I need to fix my emacs spidermonkey mode, that's what screwed me up on the case indents.  Will fix the rest, didn't make Christmas alas!  Was thinking about the range checking for the array, but that'll mean a somewhat large change... the template assumes it can do &fastclass etc., but now it would have to do &fastclasses[type], where the type id normally comes from the templated function that translates the actual typename to the enum value.  Will have to think about how to make that work.

One problem that I ran into -- it's currently impossible to construct these arrays using ConstructObject, because of this check here:

http://hg.mozilla.org/tracemonkey/file/9d0fe1237c26/js/src/jsobj.cpp#l3546

I'm not really sure how to get around this; it seems to work for arrays because there's no way to construct a non-dense array in the constructor that I can come up with, so the class type always ends up correct even if it gets switched out afterwards.

Comment 21

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: updated, v10

Converting to the array wasn't as bad as I thought -- done here.  Other nits addressed, though I didn't convert all of the JS_* -> js_* uses since a few are only used in "slow paths" and using the internal API is more verbose (e.g. JS_ValueToNumber -> js_ValueToNumber, needing to create a temporary jsval root etc.). The ones that could be done easily I did.  Hope I caught everything else...

Comment 22

[Brendan Eich [:brendan]]

Comment on attachment 419178 [details] [diff] [review]
updated, v10

>+    slice(uint32 begin, uint32 end)
>+    {
>+        if (begin > length || end > length) {
>+            return NULL;
>+        }

Nit: lose the braces.

>+    copyFrom(TypedArray *tarray)
>+    {
>+        NativeType *dest = static_cast<NativeType*>(data);
>+
>+        if (tarray->type == type) {
>+            memcpy(dest, tarray->data, tarray->byteLength);
>+            return true;
>+        }
>+
>+        switch (tarray->type) {
>+          case TypedArray::TYPE_INT8: {
>+              int8 *src = static_cast<int8*>(tarray->data);
>+              for (uintN i = 0; i < length; ++i)
>+                  *dest++ = NativeType(*src++);
>+              break;

Now inside the braces is over-indented two spaces.

>+js_IsTypedArray(JSObject *obj)
>+{
>+    return obj &&
>+           (obj->getClass() >= &TypedArray::fastClasses[0]) &&
>+           (obj->getClass() <  &TypedArray::fastClasses[TypedArray::TYPE_MAX]);

Nit: don't overparenthesize relationals or equality ops against && and ||.

r=me with these picked. Thanks, and HNY!

/be

Comment 23

[Brendan Eich [:brendan]]

(In reply to comment #20)
> One problem that I ran into -- it's currently impossible to construct these
> arrays using ConstructObject, because of this check here:
> 
> http://hg.mozilla.org/tracemonkey/file/9d0fe1237c26/js/src/jsobj.cpp#l3546
> 
> I'm not really sure how to get around this; it seems to work for arrays because
> there's no way to construct a non-dense array in the constructor that I can
> come up with, so the class type always ends up correct even if it gets switched
> out afterwards.

Please file a bug on this, maybe jorendorff will take it. We should fix it, somehow.

/be

Comment 24

[Jason Orendorff [:jorendorff]]

(In reply to comment #23)
> Please file a bug on this, maybe jorendorff will take it. We should fix it,
> somehow.

After some discussion, it looks like the check is API-visible and may even be necessary for security. Probably the best thing is to provide separate API entry points for creating typed arrays.

Comment 25

[Brendan Eich [:brendan]]

(In reply to comment #24)
> (In reply to comment #23)
> > Please file a bug on this, maybe jorendorff will take it. We should fix it,
> > somehow.
> 
> After some discussion, it looks like the check is API-visible and may even be
> necessary for security. Probably the best thing is to provide separate API
> entry points for creating typed arrays.

That's typically how we "fix" the API, precisely for compatibility (for any of a number of reasons). It requires "API GC" though ;-).

/be

Comment 26

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: updated, v10 + nits fixed

Carrying review forward -- this is v10 with the nits fixed only.

Comment 27

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: constructors followup, v1

Here's the constructors followup, along with a tiny set of things that should be rolled into the main patch.  I'll fold them both into one for landing, but didn't want to clutter the previous reviewed patch with additional changes.  I hope that my usage of JSAutoTempValueRooter is correct (and that my shortcut of using an array of N jsvals with the last one being the rval is acceptable).

Comment 28

[Jason Orendorff [:jorendorff]]

Comment on attachment 420194 [details] [diff] [review]
constructors followup, v1

In js_CreateArrayBuffer:
>+    JSAutoTempValueRooter rval(cx);
>+    if (!ArrayBuffer::class_constructor(cx, cx->globalObject,
>+                                        1, tvr.addr(), 
>+                                        rval.addr()))
>+        return NULL;
>+
>+    if (!JSVAL_IS_OBJECT(rval.value()) || JSVAL_IS_NULL(rval.value()))
>+        return NULL;

How can this happen? It seems impossible. There are similar
checks in js_CreateTypedArray, js_CreateTypedArrayWithArray, and
js_CreateTypedArrayWithBuffer.

>+    if (atype < 0 || atype >= TypedArray::TYPE_MAX) {

Please assert that the arguments are valid instead of
checking (this argument and others too).

I think js_CreateTypedArrayWithBuffer should additionally assert
that the bufArg object is actually an ArrayBuffer, just to keep
the caller honest.

r=me with those changes.

Comment 29

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

In js/src/jstypedarray.cpp and js/src/jstypedarray.h, the license block states

> * The Initial Developer of the Original Code is
> *   Mozilla Corp

This should be

* the Mozilla Foundation

(with a single space). See bug 507387.

Comment 30

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/165a48c9ea89

Comment 31

[David Mandelin [:dmandelin]]

This introduced some warnings:

../jstypedarray.cpp:1005: warning: ‘js::ArrayBuffer::jsclass’: visibility attribute ignored because it
../jstypedarray.h:58: warning: conflicts with previous declaration here

Etc. I think the JS_FRIEND_API around the class makes it visible in GCC, along with all its members. But then the static member field definition in the .cpp file is considered hidden because of the GCC pragma in config/gcc_hidden.h. It looks like these are not part of the public API so making them hidden would be good. That would require a |__attribute__((visibility ("hidden")))|, presumably as some new macro JS_PRIVATE_DATA or some such. 

Is that right?

If we really don't care about extra externals we could get rid of the warning easily by putting JS_FRIEND_DATA on the .cpp definitions.

I don't know if any of this applies to Windows.

Comment 32

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Hmm, missed that.  Those actually do need to be FRIEND, because friend code does comparisons with the specific class pointer to check whether it's a specific array type.  So putting JS_FRIEND_API on the cpp definitions is probably best.

Comment 33

[Brendan Eich [:brendan]]

JS_FRIEND_DATA

/be

Comment 34

[Eric Shepherd [:sheppy]]

We have documentation now:

https://developer.mozilla.org/en/JavaScript_typed_arrays
https://developer.mozilla.org/en/JavaScript_typed_arrays/ArrayBuffer
https://developer.mozilla.org/en/JavaScript_typed_arrays/ArrayBufferView
https://developer.mozilla.org/en/JavaScript_typed_arrays/Int8Array
https://developer.mozilla.org/en/JavaScript_typed_arrays/Uint8Array
https://developer.mozilla.org/en/JavaScript_typed_arrays/Int16Array
https://developer.mozilla.org/en/JavaScript_typed_arrays/Uint16Array
https://developer.mozilla.org/en/JavaScript_typed_arrays/Int32Array
https://developer.mozilla.org/en/JavaScript_typed_arrays/Uint32Array
https://developer.mozilla.org/en/JavaScript_typed_arrays/Float32Array
https://developer.mozilla.org/en/JavaScript_typed_arrays/Float64Array

I expect more example type stuff will come eventually in the File API, WebSockets, and WebGL documentation, but this is a good start. Please let me know if there are any issues with the documentation.