[builtins] Unified C++ implementation of Map and Set iterators.

This is the first step in optimizing Map and Set iterators. This ports
all the base functionality including

 - Set.prototype.entries
 - Set.prototype.values
 - %SetPrototypeIterator%.next
 - Map.prototype.entries
 - Map.prototype.keys
 - Map.prototype.values
 - %MapPrototypeIterator%.next

to C++ and removes all the dead code and the previous half JavaScript
implementation. The next step is to port core parts to CodeStubAssembler
and finally inline the fast-paths into TurboFan directly. The relevant
design document is at:

  https://docs.google.com/document/d/13z1fvRVpe_oEroplXEEX0a3WK94fhXorHjcOMsDmR-8

Most of this work is very similar to how the Array iterator works and we
mostly follow the same process for the implementation.

R=jgruber@chromium.org

Bug: v8:6571
Change-Id: Ieb253d6705ba4077c697a5ff0cb6f87f9c4056ff
Reviewed-on: https://chromium-review.googlesource.com/561138Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46441}

BUILD.gn

@@ -551,7 +551,6 @@ action("js2c") {
     "src/js/typedarray.js",
     "src/js/collection.js",
     "src/js/weak-collection.js",
-    "src/js/collection-iterator.js",
     "src/js/promise.js",
     "src/js/messages.js",
     "src/js/templates.js",

src/bootstrapper.cc

@@ -2971,10 +2971,18 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
     native_context()->set_map_has(*map_has);
 
     SimpleInstallFunction(prototype, "clear", Builtins::kMapClear, 0, true);
+    Handle<JSFunction> entries = SimpleInstallFunction(
+        prototype, "entries", Builtins::kMapPrototypeEntries, 0, true);
+    JSObject::AddProperty(prototype, factory->iterator_symbol(), entries,
+                          DONT_ENUM);
     SimpleInstallFunction(prototype, "forEach", Builtins::kMapForEach, 1,
                           false);
+    SimpleInstallFunction(prototype, "keys", Builtins::kMapPrototypeKeys, 0,
+                          true);
     SimpleInstallGetter(prototype, factory->InternalizeUtf8String("size"),
                         Builtins::kMapGetSize, false);
+    SimpleInstallFunction(prototype, "values", Builtins::kMapPrototypeValues, 0,
+                          true);
     InstallSpeciesGetter(js_map_fun);
   }
 
@@ -3004,10 +3012,17 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
         SimpleInstallFunction(prototype, "has", Builtins::kSetHas, 1, true);
     native_context()->set_set_has(*set_has);
     SimpleInstallFunction(prototype, "clear", Builtins::kSetClear, 0, true);
+    SimpleInstallFunction(prototype, "entries", Builtins::kSetPrototypeEntries,
+                          0, true);
     SimpleInstallFunction(prototype, "forEach", Builtins::kSetForEach, 1,
                           false);
     SimpleInstallGetter(prototype, factory->InternalizeUtf8String("size"),
                         Builtins::kSetGetSize, false);
+    Handle<JSFunction> values = SimpleInstallFunction(
+        prototype, "values", Builtins::kSetPrototypeValues, 0, true);
+    JSObject::AddProperty(prototype, factory->keys_string(), values, DONT_ENUM);
+    JSObject::AddProperty(prototype, factory->iterator_symbol(), values,
+                          DONT_ENUM);
     InstallSpeciesGetter(js_set_fun);
   }
 
@@ -3661,6 +3676,11 @@ void Bootstrapper::ExportFromRuntime(Isolate* isolate,
         prototype, factory->to_string_tag_symbol(), name,
         static_cast<PropertyAttributes>(DONT_ENUM | READ_ONLY));
 
+    // Install the next function on the {prototype}.
+    SimpleInstallFunction(prototype, "next",
+                          Builtins::kSetIteratorPrototypeNext, 0, true,
+                          kSetIteratorNext);
+
     // Setup SetIterator constructor.
     Handle<JSFunction> set_iterator_function =
         InstallFunction(container, "SetIterator", JS_SET_ITERATOR_TYPE,
@@ -3682,6 +3702,11 @@ void Bootstrapper::ExportFromRuntime(Isolate* isolate,
         prototype, factory->to_string_tag_symbol(), name,
         static_cast<PropertyAttributes>(DONT_ENUM | READ_ONLY));
 
+    // Install the next function on the {prototype}.
+    SimpleInstallFunction(prototype, "next",
+                          Builtins::kMapIteratorPrototypeNext, 0, true,
+                          kMapIteratorNext);
+
     // Setup MapIterator constructor.
     Handle<JSFunction> map_iterator_function =
         InstallFunction(container, "MapIterator", JS_MAP_ITERATOR_TYPE,

src/builtins/builtins-collections.cc

@@ -41,11 +41,9 @@ BUILTIN(MapForEach) {
   }
 
   Handle<Object> receiver = args.atOrUndefined(isolate, 2);
-  Handle<OrderedHashMap> table(OrderedHashMap::cast(map->table()));
-  Handle<JSMapIterator> iterator = isolate->factory()->NewJSMapIterator();
-  iterator->set_table(*table);
-  iterator->set_index(Smi::kZero);
-  iterator->set_kind(Smi::FromInt(JSMapIterator::kKindEntries));
+  Handle<OrderedHashMap> table(OrderedHashMap::cast(map->table()), isolate);
+  Handle<JSMapIterator> iterator = isolate->factory()->NewJSMapIterator(
+      table, 0, JSMapIterator::kKindEntries);
 
   while (iterator->HasMore()) {
     Handle<Object> key(iterator->CurrentKey(), isolate);
@@ -60,6 +58,58 @@ BUILTIN(MapForEach) {
   return isolate->heap()->undefined_value();
 }
 
+BUILTIN(MapPrototypeEntries) {
+  HandleScope scope(isolate);
+  const char* const kMethodName = "Map.prototype.entries";
+  CHECK_RECEIVER(JSMap, map, kMethodName);
+  return *isolate->factory()->NewJSMapIterator(
+      handle(OrderedHashMap::cast(map->table()), isolate), 0,
+      JSMapIterator::kKindEntries);
+}
+
+BUILTIN(MapPrototypeKeys) {
+  HandleScope scope(isolate);
+  const char* const kMethodName = "Map.prototype.keys";
+  CHECK_RECEIVER(JSMap, map, kMethodName);
+  return *isolate->factory()->NewJSMapIterator(
+      handle(OrderedHashMap::cast(map->table()), isolate), 0,
+      JSMapIterator::kKindKeys);
+}
+
+BUILTIN(MapPrototypeValues) {
+  HandleScope scope(isolate);
+  const char* const kMethodName = "Map.prototype.values";
+  CHECK_RECEIVER(JSMap, map, kMethodName);
+  return *isolate->factory()->NewJSMapIterator(
+      handle(OrderedHashMap::cast(map->table()), isolate), 0,
+      JSMapIterator::kKindValues);
+}
+
+BUILTIN(MapIteratorPrototypeNext) {
+  HandleScope scope(isolate);
+  const char* const kMethodName = "Map Iterator.prototype.next";
+  CHECK_RECEIVER(JSMapIterator, iterator, kMethodName);
+  Handle<Object> value = isolate->factory()->undefined_value();
+  bool done = true;
+  if (iterator->HasMore()) {
+    done = false;
+    switch (Smi::cast(iterator->kind())->value()) {
+      case JSMapIterator::kKindEntries:
+        value = MakeEntryPair(isolate, handle(iterator->CurrentKey(), isolate),
+                              handle(iterator->CurrentValue(), isolate));
+        break;
+      case JSMapIterator::kKindKeys:
+        value = handle(iterator->CurrentKey(), isolate);
+        break;
+      case JSMapIterator::kKindValues:
+        value = handle(iterator->CurrentValue(), isolate);
+        break;
+    }
+    iterator->MoveNext();
+  }
+  return *isolate->factory()->NewJSIteratorResult(value, done);
+}
+
 BUILTIN(SetGetSize) {
   HandleScope scope(isolate);
   const char* const kMethodName = "get Set.prototype.size";
@@ -92,11 +142,9 @@ BUILTIN(SetForEach) {
   }
 
   Handle<Object> receiver = args.atOrUndefined(isolate, 2);
-  Handle<OrderedHashSet> table(OrderedHashSet::cast(set->table()));
-  Handle<JSSetIterator> iterator = isolate->factory()->NewJSSetIterator();
-  iterator->set_table(*table);
-  iterator->set_index(Smi::kZero);
-  iterator->set_kind(Smi::FromInt(JSSetIterator::kKindValues));
+  Handle<OrderedHashSet> table(OrderedHashSet::cast(set->table()), isolate);
+  Handle<JSSetIterator> iterator = isolate->factory()->NewJSSetIterator(
+      table, 0, JSSetIterator::kKindValues);
 
   while (iterator->HasMore()) {
     Handle<Object> key(iterator->CurrentKey(), isolate);
@@ -110,5 +158,40 @@ BUILTIN(SetForEach) {
   return isolate->heap()->undefined_value();
 }
 
+BUILTIN(SetPrototypeEntries) {
+  HandleScope scope(isolate);
+  const char* const kMethodName = "Set.prototype.entries";
+  CHECK_RECEIVER(JSSet, set, kMethodName);
+  return *isolate->factory()->NewJSSetIterator(
+      handle(OrderedHashSet::cast(set->table()), isolate), 0,
+      JSSetIterator::kKindEntries);
+}
+
+BUILTIN(SetPrototypeValues) {
+  HandleScope scope(isolate);
+  const char* const kMethodName = "Set.prototype.values";
+  CHECK_RECEIVER(JSSet, set, kMethodName);
+  return *isolate->factory()->NewJSSetIterator(
+      handle(OrderedHashSet::cast(set->table()), isolate), 0,
+      JSSetIterator::kKindValues);
+}
+
+BUILTIN(SetIteratorPrototypeNext) {
+  HandleScope scope(isolate);
+  const char* const kMethodName = "Set Iterator.prototype.next";
+  CHECK_RECEIVER(JSSetIterator, iterator, kMethodName);
+  Handle<Object> value = isolate->factory()->undefined_value();
+  bool done = true;
+  if (iterator->HasMore()) {
+    value = handle(iterator->CurrentKey(), isolate);
+    done = false;
+    if (Smi::cast(iterator->kind())->value() == JSSetIterator::kKindEntries) {
+      value = MakeEntryPair(isolate, value, value);
+    }
+    iterator->MoveNext();
+  }
+  return *isolate->factory()->NewJSIteratorResult(value, done);
+}
+
 }  // namespace internal
 }  // namespace v8

src/builtins/builtins-definitions.h

@@ -583,6 +583,14 @@ namespace internal {
   CPP(MapGetSize)                                                              \
   CPP(MapClear)                                                                \
   CPP(MapForEach)                                                              \
+  /* ES #sec-map.prototype.entries */                                          \
+  CPP(MapPrototypeEntries)                                                     \
+  /* ES #sec-map.prototype.keys */                                             \
+  CPP(MapPrototypeKeys)                                                        \
+  /* ES #sec-map.prototype.values */                                           \
+  CPP(MapPrototypeValues)                                                      \
+  /* ES #sec-%mapiteratorprototype%.next */                                    \
+  CPP(MapIteratorPrototypeNext)                                                \
                                                                                \
   /* Math */                                                                   \
   /* ES6 #sec-math.abs */                                                      \
@@ -863,6 +871,12 @@ namespace internal {
   CPP(SetGetSize)                                                              \
   CPP(SetClear)                                                                \
   CPP(SetForEach)                                                              \
+  /* ES #sec-set.prototype.entries */                                          \
+  CPP(SetPrototypeEntries)                                                     \
+  /* ES #sec-set.prototype.values */                                           \
+  CPP(SetPrototypeValues)                                                      \
+  /* ES #sec-%setiteratorprototype%.next */                                    \
+  CPP(SetIteratorPrototypeNext)                                                \
                                                                                \
   /* SharedArrayBuffer */                                                      \
   CPP(SharedArrayBufferPrototypeGetByteLength)                                 \

src/compiler/typer.cc

@@ -1480,6 +1480,8 @@ Type* Typer::Visitor::JSCallTyper(Type* fun, Typer* t) {
         case kTypedArrayKeys:
         case kTypedArrayValues:
         case kArrayIteratorNext:
+        case kMapIteratorNext:
+        case kSetIteratorNext:
           return Type::OtherObject();
 
         // Array functions.

src/debug/debug-evaluate.cc

@@ -313,7 +313,6 @@ bool IntrinsicHasNoSideEffect(Runtime::FunctionId id) {
   V(FixedArrayGet)                   \
   V(StringGetRawHashField)           \
   V(GenericHash)                     \
-  V(MapIteratorInitialize)           \
   V(MapInitialize)                   \
   V(SetInitialize)                   \
   /* Called from builtins */         \
@@ -538,6 +537,9 @@ bool BuiltinHasNoSideEffect(Builtins::Name id) {
     case Builtins::kMapConstructor:
     case Builtins::kMapGet:
     case Builtins::kMapGetSize:
+    case Builtins::kMapPrototypeEntries:
+    case Builtins::kMapPrototypeKeys:
+    case Builtins::kMapPrototypeValues:
     // Math builtins.
     case Builtins::kMathAbs:
     case Builtins::kMathAcos:
@@ -590,6 +592,8 @@ bool BuiltinHasNoSideEffect(Builtins::Name id) {
     // Set builtins.
     case Builtins::kSetConstructor:
     case Builtins::kSetGetSize:
+    case Builtins::kSetPrototypeEntries:
+    case Builtins::kSetPrototypeValues:
     // String builtins. Strings are immutable.
     case Builtins::kStringFromCharCode:
     case Builtins::kStringFromCodePoint:

src/factory.cc

@@ -2078,20 +2078,28 @@ Handle<JSSet> Factory::NewJSSet() {
   return js_set;
 }
 
-
-Handle<JSMapIterator> Factory::NewJSMapIterator() {
-  Handle<Map> map(isolate()->native_context()->map_iterator_map());
-  CALL_HEAP_FUNCTION(isolate(),
-                     isolate()->heap()->AllocateJSObjectFromMap(*map),
-                     JSMapIterator);
+Handle<JSMapIterator> Factory::NewJSMapIterator(Handle<OrderedHashMap> table,
+                                                int index,
+                                                JSMapIterator::Kind kind) {
+  Handle<Map> map(isolate()->native_context()->map_iterator_map(), isolate());
+  Handle<JSMapIterator> result =
+      Handle<JSMapIterator>::cast(NewJSObjectFromMap(map));
+  result->set_table(*table);
+  result->set_index(Smi::FromInt(index));
+  result->set_kind(Smi::FromInt(kind));
+  return result;
 }
 
-
-Handle<JSSetIterator> Factory::NewJSSetIterator() {
-  Handle<Map> map(isolate()->native_context()->set_iterator_map());
-  CALL_HEAP_FUNCTION(isolate(),
-                     isolate()->heap()->AllocateJSObjectFromMap(*map),
-                     JSSetIterator);
+Handle<JSSetIterator> Factory::NewJSSetIterator(Handle<OrderedHashSet> table,
+                                                int index,
+                                                JSSetIterator::Kind kind) {
+  Handle<Map> map(isolate()->native_context()->set_iterator_map(), isolate());
+  Handle<JSSetIterator> result =
+      Handle<JSSetIterator>::cast(NewJSObjectFromMap(map));
+  result->set_table(*table);
+  result->set_index(Smi::FromInt(index));
+  result->set_kind(Smi::FromInt(kind));
+  return result;
 }
 
 ExternalArrayType Factory::GetArrayTypeFromElementsKind(ElementsKind kind) {

src/factory.h

@@ -572,9 +572,10 @@ class V8_EXPORT_PRIVATE Factory final {
   Handle<JSMap> NewJSMap();
   Handle<JSSet> NewJSSet();
 
-  // TODO(aandrey): Maybe these should take table, index and kind arguments.
-  Handle<JSMapIterator> NewJSMapIterator();
-  Handle<JSSetIterator> NewJSSetIterator();
+  Handle<JSMapIterator> NewJSMapIterator(Handle<OrderedHashMap> table,
+                                         int index, JSMapIterator::Kind kind);
+  Handle<JSSetIterator> NewJSSetIterator(Handle<OrderedHashSet> table,
+                                         int index, JSSetIterator::Kind kind);
 
   // Allocates a bound function.
   MaybeHandle<JSBoundFunction> NewJSBoundFunction(

src/js/collection-iterator.js

-// Copyright 2014 the V8 project authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-(function(global, utils) {
-
-"use strict";
-
-%CheckIsBootstrapping();
-
-// -------------------------------------------------------------------
-// Imports
-
-var GlobalMap = global.Map;
-var GlobalSet = global.Set;
-var iteratorSymbol = utils.ImportNow("iterator_symbol");
-var MapIterator = utils.ImportNow("MapIterator");
-var SetIterator = utils.ImportNow("SetIterator");
-
-// -------------------------------------------------------------------
-
-function SetIteratorConstructor(set, kind) {
-  %SetIteratorInitialize(this, set, kind);
-}
-
-
-DEFINE_METHOD(
-  SetIterator.prototype,
-  next() {
-    if (!IS_SET_ITERATOR(this)) {
-      throw %make_type_error(kIncompatibleMethodReceiver,
-                          'Set Iterator.prototype.next', this);
-    }
-
-    var value_array = [UNDEFINED, UNDEFINED];
-    var result = %_CreateIterResultObject(value_array, false);
-    switch (%SetIteratorNext(this, value_array)) {
-      case 0:
-        result.value = UNDEFINED;
-        result.done = true;
-        break;
-      case ITERATOR_KIND_VALUES:
-        result.value = value_array[0];
-        break;
-      case ITERATOR_KIND_ENTRIES:
-        value_array[1] = value_array[0];
-        break;
-    }
-
-    return result;
-  }
-);
-
-DEFINE_METHODS(
-  GlobalSet.prototype,
-  {
-    entries() {
-      if (!IS_SET(this)) {
-        throw %make_type_error(kIncompatibleMethodReceiver,
-                            'Set.prototype.entries', this);
-      }
-      return new SetIterator(this, ITERATOR_KIND_ENTRIES);
-    }
-
-    values() {
-      if (!IS_SET(this)) {
-        throw %make_type_error(kIncompatibleMethodReceiver,
-                            'Set.prototype.values', this);
-      }
-      return new SetIterator(this, ITERATOR_KIND_VALUES);
-    }
-  }
-);
-
-// -------------------------------------------------------------------
-
-%SetCode(SetIterator, SetIteratorConstructor);
-
-var SetIteratorNext = SetIterator.prototype.next;
-var SetValues = GlobalSet.prototype.values;
-
-%AddNamedProperty(GlobalSet.prototype, "keys", SetValues, DONT_ENUM);
-%AddNamedProperty(GlobalSet.prototype, iteratorSymbol, SetValues, DONT_ENUM);
-
-// -------------------------------------------------------------------
-
-function MapIteratorConstructor(map, kind) {
-  %MapIteratorInitialize(this, map, kind);
-}
-
-
-DEFINE_METHOD(
-  MapIterator.prototype,
-  next() {
-    if (!IS_MAP_ITERATOR(this)) {
-      throw %make_type_error(kIncompatibleMethodReceiver,
-                          'Map Iterator.prototype.next', this);
-    }
-
-    var value_array = [UNDEFINED, UNDEFINED];
-    var result = %_CreateIterResultObject(value_array, false);
-    switch (%MapIteratorNext(this, value_array)) {
-      case 0:
-        result.value = UNDEFINED;
-        result.done = true;
-        break;
-      case ITERATOR_KIND_KEYS:
-        result.value = value_array[0];
-        break;
-      case ITERATOR_KIND_VALUES:
-        result.value = value_array[1];
-        break;
-      // ITERATOR_KIND_ENTRIES does not need any processing.
-    }
-
-    return result;
-  }
-);
-
-
-DEFINE_METHODS(
-  GlobalMap.prototype,
-  {
-    entries() {
-      if (!IS_MAP(this)) {
-        throw %make_type_error(kIncompatibleMethodReceiver,
-                            'Map.prototype.entries', this);
-      }
-      return new MapIterator(this, ITERATOR_KIND_ENTRIES);
-    }
-
-    keys() {
-      if (!IS_MAP(this)) {
-        throw %make_type_error(kIncompatibleMethodReceiver,
-                            'Map.prototype.keys', this);
-      }
-      return new MapIterator(this, ITERATOR_KIND_KEYS);
-    }
-
-    values() {
-      if (!IS_MAP(this)) {
-        throw %make_type_error(kIncompatibleMethodReceiver,
-                            'Map.prototype.values', this);
-      }
-      return new MapIterator(this, ITERATOR_KIND_VALUES);
-    }
-  }
-);
-
-
-// -------------------------------------------------------------------
-
-%SetCode(MapIterator, MapIteratorConstructor);
-
-var MapIteratorNext = MapIterator.prototype.next;
-var MapEntries = GlobalMap.prototype.entries;
-
-%AddNamedProperty(GlobalMap.prototype, iteratorSymbol, MapEntries, DONT_ENUM);
-
-// -------------------------------------------------------------------
-// Exports
-
-utils.Export(function(to) {
-  to.MapEntries = MapEntries;
-  to.MapIteratorNext = MapIteratorNext;
-  to.SetIteratorNext = SetIteratorNext;
-  to.SetValues = SetValues;
-});
-
-})

src/objects-inl.h

@@ -6249,17 +6249,6 @@ Object* OrderedHashTableIterator<Derived, TableType>::CurrentKey() {
 }
 
 
-void JSSetIterator::PopulateValueArray(FixedArray* array) {
-  array->set(0, CurrentKey());
-}
-
-
-void JSMapIterator::PopulateValueArray(FixedArray* array) {
-  array->set(0, CurrentKey());
-  array->set(1, CurrentValue());
-}
-
-
 Object* JSMapIterator::CurrentValue() {
   OrderedHashMap* table(OrderedHashMap::cast(this->table()));
   int index = Smi::cast(this->index())->value();
@@ -6288,7 +6277,7 @@ static inline Handle<Object> MakeEntryPair(Isolate* isolate, uint32_t index,
                                                     PACKED_ELEMENTS, 2);
 }
 
-static inline Handle<Object> MakeEntryPair(Isolate* isolate, Handle<Name> key,
+static inline Handle<Object> MakeEntryPair(Isolate* isolate, Handle<Object> key,
                                            Handle<Object> value) {
   Handle<FixedArray> entry_storage =
       isolate->factory()->NewUninitializedFixedArray(2);

src/objects.cc

@@ -18552,24 +18552,6 @@ bool OrderedHashTableIterator<Derived, TableType>::HasMore() {
   return false;
 }
 
-
-template<class Derived, class TableType>
-Smi* OrderedHashTableIterator<Derived, TableType>::Next(JSArray* value_array) {
-  DisallowHeapAllocation no_allocation;
-  if (HasMore()) {
-    FixedArray* array = FixedArray::cast(value_array->elements());
-    static_cast<Derived*>(this)->PopulateValueArray(array);
-    MoveNext();
-    return Smi::cast(kind());
-  }
-  return Smi::kZero;
-}
-
-
-template Smi*
-OrderedHashTableIterator<JSSetIterator, OrderedHashSet>::Next(
-    JSArray* value_array);
-
 template bool
 OrderedHashTableIterator<JSSetIterator, OrderedHashSet>::HasMore();
 
@@ -18583,10 +18565,6 @@ template void
 OrderedHashTableIterator<JSSetIterator, OrderedHashSet>::Transition();
 
 
-template Smi*
-OrderedHashTableIterator<JSMapIterator, OrderedHashMap>::Next(
-    JSArray* value_array);
-
 template bool
 OrderedHashTableIterator<JSMapIterator, OrderedHashMap>::HasMore();
 

src/objects.h

@@ -4662,6 +4662,8 @@ enum BuiltinFunctionId {
   kArrayKeys,
   kArrayValues,
   kArrayIteratorNext,
+  kMapIteratorNext,
+  kSetIteratorNext,
   kDataViewBuffer,
   kDataViewByteLength,
   kDataViewByteOffset,

src/objects/hash-table.h

@@ -877,11 +877,6 @@ class OrderedHashTableIterator : public JSObject {
   // Move the index forward one.
   void MoveNext() { set_index(Smi::FromInt(Smi::cast(index())->value() + 1)); }
 
-  // Populates the array with the next key and value and then moves the iterator
-  // forward.
-  // This returns the |kind| or 0 if the iterator is already at the end.
-  Smi* Next(JSArray* value_array);
-
   // Returns the current key of the iterator. This should only be called when
   // |HasMore| returns true.
   inline Object* CurrentKey();
@@ -903,10 +898,6 @@ class JSSetIterator
 
   DECL_CAST(JSSetIterator)
 
-  // Called by |Next| to populate the array. This allows the subclasses to
-  // populate the array differently.
-  inline void PopulateValueArray(FixedArray* array);
-
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(JSSetIterator);
 };
@@ -920,10 +911,6 @@ class JSMapIterator
 
   DECL_CAST(JSMapIterator)
 
-  // Called by |Next| to populate the array. This allows the subclasses to
-  // populate the array differently.
-  inline void PopulateValueArray(FixedArray* array);
-
   // Returns the current value of the iterator. This should only be called when
   // |HasMore| returns true.
   inline Object* CurrentValue();

src/runtime/runtime-collections.cc

@@ -75,45 +75,16 @@ RUNTIME_FUNCTION(Runtime_SetShrink) {
   return isolate->heap()->undefined_value();
 }
 
-RUNTIME_FUNCTION(Runtime_SetIteratorInitialize) {
-  HandleScope scope(isolate);
-  DCHECK_EQ(3, args.length());
-  CONVERT_ARG_HANDLE_CHECKED(JSSetIterator, holder, 0);
-  CONVERT_ARG_HANDLE_CHECKED(JSSet, set, 1);
-  CONVERT_SMI_ARG_CHECKED(kind, 2)
-  CHECK(kind == JSSetIterator::kKindValues ||
-        kind == JSSetIterator::kKindEntries);
-  Handle<OrderedHashSet> table(OrderedHashSet::cast(set->table()));
-  holder->set_table(*table);
-  holder->set_index(Smi::kZero);
-  holder->set_kind(Smi::FromInt(kind));
-  return isolate->heap()->undefined_value();
-}
-
-
 RUNTIME_FUNCTION(Runtime_SetIteratorClone) {
   HandleScope scope(isolate);
   DCHECK_EQ(1, args.length());
   CONVERT_ARG_HANDLE_CHECKED(JSSetIterator, holder, 0);
-
-  Handle<JSSetIterator> result = isolate->factory()->NewJSSetIterator();
-  result->set_table(holder->table());
-  result->set_index(Smi::FromInt(Smi::cast(holder->index())->value()));
-  result->set_kind(Smi::FromInt(Smi::cast(holder->kind())->value()));
-
-  return *result;
-}
-
-
-RUNTIME_FUNCTION(Runtime_SetIteratorNext) {
-  SealHandleScope shs(isolate);
-  DCHECK_EQ(2, args.length());
-  CONVERT_ARG_CHECKED(JSSetIterator, holder, 0);
-  CONVERT_ARG_CHECKED(JSArray, value_array, 1);
-  return holder->Next(value_array);
+  return *isolate->factory()->NewJSSetIterator(
+      handle(OrderedHashSet::cast(holder->table()), isolate),
+      Smi::cast(holder->index())->value(),
+      static_cast<JSSetIterator::Kind>(Smi::cast(holder->kind())->value()));
 }
 
-
 // The array returned contains the following information:
 // 0: HasMore flag
 // 1: Iteration index
@@ -159,35 +130,14 @@ RUNTIME_FUNCTION(Runtime_MapGrow) {
   return isolate->heap()->undefined_value();
 }
 
-
-RUNTIME_FUNCTION(Runtime_MapIteratorInitialize) {
-  HandleScope scope(isolate);
-  DCHECK_EQ(3, args.length());
-  CONVERT_ARG_HANDLE_CHECKED(JSMapIterator, holder, 0);
-  CONVERT_ARG_HANDLE_CHECKED(JSMap, map, 1);
-  CONVERT_SMI_ARG_CHECKED(kind, 2)
-  CHECK(kind == JSMapIterator::kKindKeys ||
-        kind == JSMapIterator::kKindValues ||
-        kind == JSMapIterator::kKindEntries);
-  Handle<OrderedHashMap> table(OrderedHashMap::cast(map->table()));
-  holder->set_table(*table);
-  holder->set_index(Smi::kZero);
-  holder->set_kind(Smi::FromInt(kind));
-  return isolate->heap()->undefined_value();
-}
-
-
 RUNTIME_FUNCTION(Runtime_MapIteratorClone) {
   HandleScope scope(isolate);
   DCHECK_EQ(1, args.length());
   CONVERT_ARG_HANDLE_CHECKED(JSMapIterator, holder, 0);
-
-  Handle<JSMapIterator> result = isolate->factory()->NewJSMapIterator();
-  result->set_table(holder->table());
-  result->set_index(Smi::FromInt(Smi::cast(holder->index())->value()));
-  result->set_kind(Smi::FromInt(Smi::cast(holder->kind())->value()));
-
-  return *result;
+  return *isolate->factory()->NewJSMapIterator(
+      handle(OrderedHashMap::cast(holder->table()), isolate),
+      Smi::cast(holder->index())->value(),
+      static_cast<JSMapIterator::Kind>(Smi::cast(holder->kind())->value()));
 }
 
 
@@ -216,16 +166,6 @@ RUNTIME_FUNCTION(Runtime_GetWeakMapEntries) {
   return *JSWeakCollection::GetEntries(holder, max_entries);
 }
 
-
-RUNTIME_FUNCTION(Runtime_MapIteratorNext) {
-  SealHandleScope shs(isolate);
-  DCHECK_EQ(2, args.length());
-  CONVERT_ARG_CHECKED(JSMapIterator, holder, 0);
-  CONVERT_ARG_CHECKED(JSArray, value_array, 1);
-  return holder->Next(value_array);
-}
-
-
 RUNTIME_FUNCTION(Runtime_WeakCollectionInitialize) {
   HandleScope scope(isolate);
   DCHECK_EQ(1, args.length());

src/runtime/runtime.h

@@ -97,18 +97,14 @@ namespace internal {
   F(SetInitialize, 1, 1)                  \
   F(SetGrow, 1, 1)                        \
   F(SetShrink, 1, 1)                      \
-  F(SetIteratorInitialize, 3, 1)          \
   F(SetIteratorClone, 1, 1)               \
-  F(SetIteratorNext, 2, 1)                \
   F(SetIteratorDetails, 1, 1)             \
   F(MapInitialize, 1, 1)                  \
   F(MapShrink, 1, 1)                      \
   F(MapGrow, 1, 1)                        \
-  F(MapIteratorInitialize, 3, 1)          \
   F(MapIteratorClone, 1, 1)               \
   F(MapIteratorDetails, 1, 1)             \
   F(GetWeakMapEntries, 2, 1)              \
-  F(MapIteratorNext, 2, 1)                \
   F(WeakCollectionInitialize, 1, 1)       \
   F(WeakCollectionGet, 3, 1)              \
   F(WeakCollectionHas, 3, 1)              \

src/v8.gyp

@@ -2335,7 +2335,6 @@
           'js/typedarray.js',
           'js/collection.js',
           'js/weak-collection.js',
-          'js/collection-iterator.js',
           'js/promise.js',
           'js/messages.js',
           'js/templates.js',