Optimize new TypedArray(typedArray) constructor

src/js/typedarray.js

Index: src/js/typedarray.js
diff --git a/src/js/typedarray.js b/src/js/typedarray.js
index b16dab2237f869a0645b114a75e0aa3949fc68f1..9cb5b6e9a07eef617e0d068558b88a5a9ab512f7 100644
--- a/src/js/typedarray.js
+++ b/src/js/typedarray.js
@@ -202,25 +202,18 @@ function NAMEConstructByLength(obj, length) {
 
 function NAMEConstructByArrayLike(obj, arrayLike) {
   var length = arrayLike.length;
+
   var l = ToPositiveInteger(length, kInvalidTypedArrayLength);
 
   if (l > %_MaxSmi()) {
     throw MakeRangeError(kInvalidTypedArrayLength);
   }
-  var initialized = false;
   var byteLength = l * ELEMENT_SIZE;
-  if (byteLength <= %_TypedArrayMaxSizeInHeap()) {

[adamk, 2016/03/08 19:32:10]

What's up with this if statement? Why is it OK to remove it, that is?

[Dan Ehrenberg, 2016/03/08 19:55:55]

Apparently it wasn't, and this was found by the tests! The new version leaves
this function as-is, except for getting length passed in from the outside. I
thought somehow it only mattered for TypedArrays, but it matters for arraylikes
also.

-    %_TypedArrayInitialize(obj, ARRAY_ID, null, 0, byteLength, false);
-  } else {
-    initialized =
-        %TypedArrayInitializeFromArrayLike(obj, ARRAY_ID, arrayLike, l);
-  }
-  if (!initialized) {
-    for (var i = 0; i < l; i++) {
-      // It is crucial that we let any execptions from arrayLike[i]
-      // propagate outside the function.
-      obj[i] = arrayLike[i];
-    }
+  %_TypedArrayInitialize(obj, ARRAY_ID, null, 0, byteLength, false);
+  for (var i = 0; i < l; i++) {
+    // It is crucial that we let any exceptions from arrayLike[i]

[adamk, 2016/03/08 19:32:10]

Not sure this comment is worth keeping around.

[Dan Ehrenberg, 2016/03/08 19:55:54]

How about I leave all this as is for now, since I'm not changing this part
anymore, and come back later when I do the backlog of TypedArray cleanups I've
been thinking about?

[adamk, 2016/03/08 20:05:39]

Fine to leave for now since you're not touching it anymore.

+    // propagate outside the function.
+    obj[i] = arrayLike[i];
   }
 }
 
@@ -251,20 +244,17 @@ function NAMEConstructByTypedArray(obj, typedArray) {
   var length = %_TypedArrayGetLength(typedArray);
   var byteLength = %_ArrayBufferViewGetByteLength(typedArray);
   var newByteLength = length * ELEMENT_SIZE;
+  var initialized = %TypedArrayInitializeFromArrayLike(obj, ARRAY_ID, typedArray, length);

[adamk, 2016/03/08 19:32:10]

Will this ever return false? If not, and this is the only remaining caller,
should it be changed to %TypedArrayInitializeFromTypedArray?

Also, 80 col nit.

[Dan Ehrenberg, 2016/03/08 19:55:54]

I played around with a bunch of alternatives here, including changing around
what the runtime function does. I ended up reverting to a more conservative path
once I realized how the previous code was taking advantage of the runtime
function in the arraylike case (not redundantly zeroing and initializing, which
is OK because an uninitialized ArrayBuffer never escapes, even if an exception
is thrown when setting properties).

   var bufferConstructor = SpeciesConstructor(srcData, GlobalArrayBuffer);
-  var data = new GlobalArrayBuffer(newByteLength);
   var prototype = bufferConstructor.prototype;
   // TODO(littledan): Use the right prototype based on bufferConstructor's realm
   if (IS_RECEIVER(prototype) && prototype !== GlobalArrayBufferPrototype) {
-    %InternalSetPrototype(data, prototype);
+    %InternalSetPrototype(%TypedArrayGetBuffer(obj), prototype);
   }
-  %_TypedArrayInitialize(obj, ARRAY_ID, data, 0, newByteLength, true);
-  // Note: The separate CloneArrayBuffer path in the spec ensures
-  // that NaNs are not canonicalized, but because V8 does not
-  // canonicalize NaNs, we do not have to do anything different.
-  // TODO(littledan): Make a fastpath based on memcpy
-  for (var i = 0; i < length; i++) {
-    obj[i] = typedArray[i];
+  if (!initialized) {
+    for (var i = 0; i < length; i++) {
+      obj[i] = typedArray[i];
+    }
   }
 }