Hack forEach to use HasFastPackedElements

src/array.js

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 1209 matching lines @@
       if (i in array) {
         var element = array[i];
         %_CallFunction(receiver, element, i, array, f);
       }
     }
     // End of duplicate.
   }
 }
 
 
+function ArrayForEach2(f, receiver) {
+  if (IS_NULL_OR_UNDEFINED(this) && !IS_UNDETECTABLE(this)) {
+    throw MakeTypeError("called_on_null_or_undefined",
+                        ["Array.prototype.forEach"]);
+  }
+
+  // Pull out the length so that modifications to the length in the
+  // loop will not affect the looping and side effects are visible.
+  var array = ToObject(this);
+  var length = TO_UINT32(array.length);
+
+  if (!IS_SPEC_FUNCTION(f)) {
+    throw MakeTypeError('called_non_callable', [ f ]);
+  }
+  if (IS_NULL_OR_UNDEFINED(receiver)) {
+    receiver = %GetDefaultReceiver(f) || receiver;
+  } else if (!IS_SPEC_OBJECT(receiver) && %IsClassicModeFunction(f)) {
+    receiver = ToObject(receiver);
+  }
+
+  if (%DebugCallbackSupportsStepping(f)) {
+    for (var i = 0; i < length; i++) {
+      if (i in array) {
+        var element = array[i];
+        // Prepare break slots for debugger step in.
+        %DebugPrepareStepInIfStepping(f);
+        %_CallFunction(receiver, element, i, array, f);
+      }
+    }
+  } else {
+    // This is mostly a duplicate of the previous loop sans debug stepping.
+    var isProxy = %IsJSProxy(array);
+    for (var i = 0; i < length; i++) {
+      if (!isProxy && %HasFastPackedElements(array) || i in array) {

[Michael Starzinger, 2013/08/13 14:10:22]

It is _really_ surprising that the runtime call into
Runtime_HasFastPackedElements is cheaper than the "in" keyword. This suggests
that there is a lot of potential in actually improving "in" in general. This
needs investigation of the generated code!

[Michael Starzinger, 2013/08/13 15:47:16]

Now that I think about it, I don't actually think that this is a valid
optimization either. What happens if the callback actually changes the array in
a way so that it remains in fast-packed mode, but "i" still runs out of bounds.
Imagine the following callback.

var array = [1,2,3,4,5,6];
Object.defineProperty(array, '1', {
  get: function () { array.length = 3; },
  configurable: true
});
array.forEach(f);

[Toon Verwaest, 2013/08/13 20:47:42]

I don't think you can have indexed accessors while being packed. I think we
always go dictionary mode when accessors are installed. Holey arrays would have
this problem; but they are already excluded explicitly.

On 2013/08/13 15:47:16, Michael Starzinger wrote:

[Michael Starzinger, 2013/08/14 07:48:47]

OK, admittedly, my example does not trigger the problem and is unnecessarily
complex. But you don't need accessors to reproduce this problem ...

var array = [1,2,3,4,5,6];
function f(value, index, array) {
  array.length = 3;
}
array.forEach(f);

+        var element = array[i];
+        %_CallFunction(receiver, element, i, array, f);
+      }
+    }
+    // End of duplicate.
+  }
+}
+
+
+function ArrayForEach3(f, receiver) {
+  if (IS_NULL_OR_UNDEFINED(this) && !IS_UNDETECTABLE(this)) {
+    throw MakeTypeError("called_on_null_or_undefined",
+                        ["Array.prototype.forEach"]);
+  }
+
+  // Pull out the length so that modifications to the length in the
+  // loop will not affect the looping and side effects are visible.
+  var array = ToObject(this);
+  var length = TO_UINT32(array.length);
+
+  if (!IS_SPEC_FUNCTION(f)) {
+    throw MakeTypeError('called_non_callable', [ f ]);
+  }
+  if (IS_NULL_OR_UNDEFINED(receiver)) {
+    receiver = %GetDefaultReceiver(f) || receiver;
+  } else if (!IS_SPEC_OBJECT(receiver) && %IsClassicModeFunction(f)) {
+    receiver = ToObject(receiver);
+  }
+
+  if (%DebugCallbackSupportsStepping(f)) {
+    for (var i = 0; i < length; i++) {
+      if (i in array) {
+        var element = array[i];
+        // Prepare break slots for debugger step in.
+        %DebugPrepareStepInIfStepping(f);
+        %_CallFunction(receiver, element, i, array, f);
+      }
+    }
+  } else if (%IsJSProxy(array)) {
+    // This is a duplicate of the previous loop sans debug stepping.
+    for (var i = 0; i < length; i++) {
+      if (i in array) {
+        var element = array[i];
+        %_CallFunction(receiver, element, i, array, f);
+      }
+    }
+  } else {
+    // This is mostly a duplicate of the previous loop but since the order is
+    // not observable we can optimize further.
+    for (var i = 0; i < length; i++) {
+      var element = array[i];
+      if (!IS_UNDEFINED(element) || i in array) {

[Michael Starzinger, 2013/08/13 14:10:22]

This is actually wrong as the i-th element might have a getter that removes the
element upon first access but still returns undefined. This will cause us to do
the "in" check on the property after the element has been removed.

We had this "optimization" for a while and it was removed. The test262 suite
should actually cover this case.

[Michael Starzinger, 2013/08/13 14:18:58]

This is the issue tracking exactly this:

https://code.google.com/p/v8/issues/detail?id=1956

+        %_CallFunction(receiver, element, i, array, f);
+      }
+    }
+    // End of duplicate.
+  }
+}
+
+
 // Executes the function once for each element present in the
 // array until it finds one where callback returns true.
 function ArraySome(f, receiver) {
   if (IS_NULL_OR_UNDEFINED(this) && !IS_UNDETECTABLE(this)) {
     throw MakeTypeError("called_on_null_or_undefined",
                         ["Array.prototype.some"]);
   }
 
   // Pull out the length so that modifications to the length in the
   // loop will not affect the looping and side effects are visible.
@@ skipping 308 matching lines @@
       current = array[i];
       if (!IS_UNDEFINED(current) || i in array) {
         i--;
         break find_initial;
       }
     }
     throw MakeTypeError('reduce_no_initial', []);
   }
 
   var receiver = %GetDefaultReceiver(callback);
-
   if (%DebugCallbackSupportsStepping(callback)) {
     for (; i >= 0; i--) {
       if (i in array) {
         var element = array[i];
         // Prepare break slots for debugger step in.
         %DebugPrepareStepInIfStepping(callback);
         current =
           %_CallFunction(receiver, current, element, i, array, callback);
       }
     }
@@ skipping 56 matching lines @@
     "push", getFunction("push", ArrayPush, 1),
     "concat", getFunction("concat", ArrayConcat, 1),
     "reverse", getFunction("reverse", ArrayReverse),
     "shift", getFunction("shift", ArrayShift),
     "unshift", getFunction("unshift", ArrayUnshift, 1),
     "slice", getFunction("slice", ArraySlice, 2),
     "splice", getFunction("splice", ArraySplice, 2),
     "sort", getFunction("sort", ArraySort),
     "filter", getFunction("filter", ArrayFilter, 1),
     "forEach", getFunction("forEach", ArrayForEach, 1),
+    "forEach2", getFunction("forEach2", ArrayForEach2, 1),
+    "forEach3", getFunction("forEach3", ArrayForEach3, 1),
     "some", getFunction("some", ArraySome, 1),
     "every", getFunction("every", ArrayEvery, 1),
     "map", getFunction("map", ArrayMap, 1),
     "indexOf", getFunction("indexOf", ArrayIndexOf, 1),
     "lastIndexOf", getFunction("lastIndexOf", ArrayLastIndexOf, 1),
     "reduce", getFunction("reduce", ArrayReduce, 1),
     "reduceRight", getFunction("reduceRight", ArrayReduceRight, 1)
   ));
 
   %FinishArrayPrototypeSetup($Array.prototype);
@@ skipping 11 matching lines @@
   ));
 
   SetUpLockedPrototype(InternalPackedArray, $Array(), $Array(
     "join", getFunction("join", ArrayJoin),
     "pop", getFunction("pop", ArrayPop),
     "push", getFunction("push", ArrayPush)
   ));
 }
 
 SetUpArray();