559476 - Do a fast copy of dense primitive arrays when making a structured clone

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v1

For big data arrays it would be better to quickly copy rather than enumerate item by item. This only works if the array is dense and is made up of primitives. As soon as we have an object in the array we have to bail and enumerate anyway.

Comment 1

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v1.1

This removes the memcopy in favor of walking the array only once.

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v2

Updated with fixes suggested by jorendorff and vlad.

Comment 3

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v3

Updated with comments from mrbkap.

Comment 4

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v3.1

This is nearly the same but uses memcpy instead of walking the array twice.

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 440503 [details] [diff] [review]
Patch, v3.1

I talked with bent on IRC about this patch. He has a new patch coming.

Comment 6

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v3.2

One more

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 440640 [details] [diff] [review]
Patch, v3.2

Great, thanks.

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 440640 [details] [diff] [review]
Patch, v3.2

>+  JSObject* newArray;
>+  if (js_CloneDensePrimitiveArray(cx, obj, &newArray) && newArray) {
>+    return SetPropertyOnValueOrObject(cx, OBJECT_TO_JSVAL(newArray), rval, robj,
>+                                      rid);
>+  }

I realized after I marked this: if js_CloneDensePrimitiveArray returns false, then you need to return failure, otherwise you could leave an exception unreported.

Comment 9

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Ah true. I'll fix before checkin.

Comment 10

[timeless]

http://hg.mozilla.org/mozilla-central/rev/2e62de00c6a3
this also fixed bug 562513