Optimize Map/Set.prototype.forEach

src/collection.js

Index: src/collection.js
diff --git a/src/collection.js b/src/collection.js
index 0d8dd77f7bd3be944ffcd25b51544d8ebe740c00..ca31152d89d5a5410f2f24203872269d9b9441c1 100644
--- a/src/collection.js
+++ b/src/collection.js
@@ -80,11 +80,13 @@ function SetForEach(f, receiver) {
   }
 
   var iterator = new SetIterator(this, ITERATOR_KIND_VALUES);
-  var entry;
+  var key;
   var stepping = %_DebugCallbackSupportsStepping(f);
-  while (!(entry = %SetIteratorNext(iterator)).done) {
+  while (%SetIteratorHasMore(iterator)) {
+    key = %SetIteratorCurrentKey(iterator);
+    %SetIteratorMoveNext(iterator);

[Michael Starzinger, 2014/06/23 19:51:52]

I am _very_ surprised that this is faster. How are three runtime calls faster
than one? Don't get me wrong, I trust your measurements, but is this a good
optimization in the long run?

[arv (Not doing code reviews), 2014/06/23 20:31:51]

The cost of 1 call vs 3 calls is not so large. Since we have 1 this code does
not get optimized so we are already paying the overhead of runtime calls, this
just makes that overhead 3x larger (same order of magnitude).

The main reason why this is faster is that creating the iterator result object
and then extracting the data is a lot of overhead. For Map.prototype.forEach we
also had to create temporary array and extract that data from that too.

     if (stepping) %DebugPrepareStepInIfStepping(f);
-    %_CallFunction(receiver, entry.value, entry.value, this, f);
+    %_CallFunction(receiver, key, key, this, f);
   }
 }
 
@@ -191,17 +193,21 @@ function MapForEach(f, receiver) {
   }
 
   var iterator = new MapIterator(this, ITERATOR_KIND_ENTRIES);
-  var entry;
+  var key, value;
   var stepping = %_DebugCallbackSupportsStepping(f);
-  while (!(entry = %MapIteratorNext(iterator)).done) {
+  while (%MapIteratorHasMore(iterator)) {
+    key = %MapIteratorCurrentKey(iterator);
+    value = %MapIteratorCurrentValue(iterator);
+    %MapIteratorMoveNext(iterator);
     if (stepping) %DebugPrepareStepInIfStepping(f);
-    %_CallFunction(receiver, entry.value[1], entry.value[0], this, f);
+    %_CallFunction(receiver, value, key, this, f);
   }
 }
 
 
 // -------------------------------------------------------------------
 
+
 function SetUpMap() {
   %CheckIsBootstrapping();