[builtins] Port {Map,Set}.prototype.forEach to CSA.

This generalizes the existing support for Map and Set iteration in the
CSA a bit and makes it possible to reuse the logic to implement forEach
as well. It also introduces an empty_ordered_hash_table, which is used
as a sentinel for exhausted iterators to avoid the need to deal with
undefined there as well (not observable from JavaScript).

TBR=ulan@chromium.org
R=jgruber@chromium.org

Bug: v8:5269, v8:5717
Change-Id: Ifb9ec5ecb20939aa9b7d2471537f8ccd4af04c8f
Reviewed-on: https://chromium-review.googlesource.com/565260Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46547}

src/bootstrapper.cc

@@ -2915,8 +2915,8 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
         prototype, "entries", Builtins::kMapPrototypeEntries, 0, true);
     JSObject::AddProperty(prototype, factory->iterator_symbol(), entries,
                           DONT_ENUM);
-    SimpleInstallFunction(prototype, "forEach", Builtins::kMapForEach, 1,
-                          false);
+    SimpleInstallFunction(prototype, "forEach", Builtins::kMapPrototypeForEach,
+                          1, false);
     SimpleInstallFunction(prototype, "keys", Builtins::kMapPrototypeKeys, 0,
                           true);
     SimpleInstallGetter(prototype, factory->InternalizeUtf8String("size"),
@@ -2954,8 +2954,8 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
     SimpleInstallFunction(prototype, "clear", Builtins::kSetClear, 0, true);
     SimpleInstallFunction(prototype, "entries", Builtins::kSetPrototypeEntries,
                           0, true);
-    SimpleInstallFunction(prototype, "forEach", Builtins::kSetForEach, 1,
-                          false);
+    SimpleInstallFunction(prototype, "forEach", Builtins::kSetPrototypeForEach,
+                          1, false);
     SimpleInstallGetter(prototype, factory->InternalizeUtf8String("size"),
                         Builtins::kSetGetSize, false);
     Handle<JSFunction> values = SimpleInstallFunction(

src/builtins/builtins-collections-gen.cc

@@ -32,8 +32,16 @@ class CollectionsBuiltinsAssembler : public CodeStubAssembler {
   template <typename CollectionType, int entrysize>
   Node* CallHasRaw(Node* const table, Node* const key);
 
+  // Transitions the iterator to the non obsolete backing store.
+  // This is a NOP if the [table] is not obsolete.
+  typedef std::function<void(Node* const table, Node* const index)>
+      UpdateInTransition;
+  template <typename TableType>
+  std::tuple<Node*, Node*> Transition(
+      Node* const table, Node* const index,
+      UpdateInTransition const& update_in_transition);
   template <typename IteratorType, typename TableType>
-  std::tuple<Node*, Node*> Transition(Node* const iterator);
+  std::tuple<Node*, Node*> TransitionAndUpdate(Node* const iterator);
   template <typename TableType>
   std::tuple<Node*, Node*, Node*> NextSkipHoles(Node* table, Node* index,
                                                 Label* if_end);
@@ -648,13 +656,12 @@ void CollectionsBuiltinsAssembler::FindOrderedHashMapEntry(
   }
 }
 
-template <typename IteratorType, typename TableType>
+template <typename TableType>
 std::tuple<Node*, Node*> CollectionsBuiltinsAssembler::Transition(
-    Node* const iterator) {
-  VARIABLE(var_table, MachineRepresentation::kTagged,
-           LoadObjectField(iterator, IteratorType::kTableOffset));
-  VARIABLE(var_index, MachineType::PointerRepresentation(),
-           LoadAndUntagObjectField(iterator, IteratorType::kIndexOffset));
+    Node* const table, Node* const index,
+    UpdateInTransition const& update_in_transition) {
+  VARIABLE(var_index, MachineType::PointerRepresentation(), index);
+  VARIABLE(var_table, MachineRepresentation::kTagged, table);
   Label if_done(this), if_transition(this, Label::kDeferred);
   Branch(TaggedIsSmi(
              LoadObjectField(var_table.value(), TableType::kNextTableOffset)),
@@ -711,10 +718,8 @@ std::tuple<Node*, Node*> CollectionsBuiltinsAssembler::Transition(
     }
     BIND(&done_loop);
 
-    // Update the {iterator} with the new state.
-    StoreObjectField(iterator, IteratorType::kTableOffset, var_table.value());
-    StoreObjectFieldNoWriteBarrier(iterator, IteratorType::kIndexOffset,
-                                   SmiTag(var_index.value()));
+    // Update with the new {table} and {index}.
+    update_in_transition(var_table.value(), var_index.value());
     Goto(&if_done);
   }
 
@@ -722,6 +727,20 @@ std::tuple<Node*, Node*> CollectionsBuiltinsAssembler::Transition(
   return std::tuple<Node*, Node*>(var_table.value(), var_index.value());
 }
 
+template <typename IteratorType, typename TableType>
+std::tuple<Node*, Node*> CollectionsBuiltinsAssembler::TransitionAndUpdate(
+    Node* const iterator) {
+  return Transition<TableType>(
+      LoadObjectField(iterator, IteratorType::kTableOffset),
+      LoadAndUntagObjectField(iterator, IteratorType::kIndexOffset),
+      [this, iterator](Node* const table, Node* const index) {
+        // Update the {iterator} with the new state.
+        StoreObjectField(iterator, IteratorType::kTableOffset, table);
+        StoreObjectFieldNoWriteBarrier(iterator, IteratorType::kIndexOffset,
+                                       SmiTag(index));
+      });
+}
+
 template <typename TableType>
 std::tuple<Node*, Node*, Node*> CollectionsBuiltinsAssembler::NextSkipHoles(
     Node* table, Node* index, Label* if_end) {
@@ -809,6 +828,69 @@ TF_BUILTIN(MapPrototypeEntries, CollectionsBuiltinsAssembler) {
       context, Context::MAP_KEY_VALUE_ITERATOR_MAP_INDEX, receiver));
 }
 
+TF_BUILTIN(MapPrototypeForEach, CollectionsBuiltinsAssembler) {
+  const char* const kMethodName = "Map.prototype.forEach";
+  Node* const argc = Parameter(BuiltinDescriptor::kArgumentsCount);
+  Node* const context = Parameter(BuiltinDescriptor::kContext);
+  CodeStubArguments args(this, ChangeInt32ToIntPtr(argc));
+  Node* const receiver = args.GetReceiver();
+  Node* const callback = args.GetOptionalArgumentValue(0);
+  Node* const this_arg = args.GetOptionalArgumentValue(1);
+
+  ThrowIfNotInstanceType(context, receiver, JS_MAP_TYPE, kMethodName);
+
+  // Ensure that {callback} is actually callable.
+  Label callback_not_callable(this, Label::kDeferred);
+  GotoIf(TaggedIsSmi(callback), &callback_not_callable);
+  GotoIfNot(IsCallable(callback), &callback_not_callable);
+
+  VARIABLE(var_index, MachineType::PointerRepresentation(), IntPtrConstant(0));
+  VARIABLE(var_table, MachineRepresentation::kTagged,
+           LoadObjectField(receiver, JSMap::kTableOffset));
+  Label loop(this, {&var_index, &var_table}), done_loop(this);
+  Goto(&loop);
+  BIND(&loop);
+  {
+    // Transition {table} and {index} if there was any modification to
+    // the {receiver} while we're iterating.
+    Node* index = var_index.value();
+    Node* table = var_table.value();
+    std::tie(table, index) =
+        Transition<OrderedHashMap>(table, index, [](Node*, Node*) {});
+
+    // Read the next entry from the {table}, skipping holes.
+    Node* entry_key;
+    Node* entry_start_position;
+    std::tie(entry_key, entry_start_position, index) =
+        NextSkipHoles<OrderedHashMap>(table, index, &done_loop);
+
+    // Load the entry value as well.
+    Node* entry_value = LoadFixedArrayElement(
+        table, entry_start_position,
+        (OrderedHashMap::kHashTableStartIndex + OrderedHashMap::kValueOffset) *
+            kPointerSize);
+
+    // Invoke the {callback} passing the {entry_key}, {entry_value} and the
+    // {receiver}.
+    CallJS(CodeFactory::Call(isolate()), context, callback, this_arg,
+           entry_value, entry_key, receiver);
+
+    // Continue with the next entry.
+    var_index.Bind(index);
+    var_table.Bind(table);
+    Goto(&loop);
+  }
+
+  BIND(&done_loop);
+  args.PopAndReturn(UndefinedConstant());
+
+  BIND(&callback_not_callable);
+  {
+    CallRuntime(Runtime::kThrowCalledNonCallable, context, callback);
+    Unreachable();
+  }
+}
+
 TF_BUILTIN(MapPrototypeKeys, CollectionsBuiltinsAssembler) {
   Node* const receiver = Parameter(Descriptor::kReceiver);
   Node* const context = Parameter(Descriptor::kContext);
@@ -847,19 +929,16 @@ TF_BUILTIN(MapIteratorPrototypeNext, CollectionsBuiltinsAssembler) {
   BIND(&if_receiver_valid);
 
   // Check if the {receiver} is exhausted.
-  Label return_value(this), return_entry(this),
-      return_end(this, Label::kDeferred);
-  VARIABLE(var_value, MachineRepresentation::kTagged, UndefinedConstant());
   VARIABLE(var_done, MachineRepresentation::kTagged, TrueConstant());
-  // TODO(bmeurer): Don't stick undefined in here, but some canonical
-  // empty_table, to avoid this check.
-  GotoIf(IsUndefined(LoadObjectField(receiver, JSMapIterator::kTableOffset)),
-         &return_value);
+  VARIABLE(var_value, MachineRepresentation::kTagged, UndefinedConstant());
+  Label return_value(this, {&var_done, &var_value}), return_entry(this),
+      return_end(this, Label::kDeferred);
 
   // Transition the {receiver} table if necessary.
   Node* table;
   Node* index;
-  std::tie(table, index) = Transition<JSMapIterator, OrderedHashMap>(receiver);
+  std::tie(table, index) =
+      TransitionAndUpdate<JSMapIterator, OrderedHashMap>(receiver);
 
   // Read the next entry from the {table}, skipping holes.
   Node* entry_key;
@@ -898,8 +977,7 @@ TF_BUILTIN(MapIteratorPrototypeNext, CollectionsBuiltinsAssembler) {
   BIND(&return_end);
   {
     StoreObjectFieldRoot(receiver, JSMapIterator::kTableOffset,
-                         Heap::kUndefinedValueRootIndex);
-    var_value.Bind(UndefinedConstant());
+                         Heap::kEmptyOrderedHashTableRootIndex);
     Goto(&return_value);
   }
 }
@@ -924,6 +1002,62 @@ TF_BUILTIN(SetPrototypeEntries, CollectionsBuiltinsAssembler) {
       context, Context::SET_KEY_VALUE_ITERATOR_MAP_INDEX, receiver));
 }
 
+TF_BUILTIN(SetPrototypeForEach, CollectionsBuiltinsAssembler) {
+  const char* const kMethodName = "Set.prototype.forEach";
+  Node* const argc = Parameter(BuiltinDescriptor::kArgumentsCount);
+  Node* const context = Parameter(BuiltinDescriptor::kContext);
+  CodeStubArguments args(this, ChangeInt32ToIntPtr(argc));
+  Node* const receiver = args.GetReceiver();
+  Node* const callback = args.GetOptionalArgumentValue(0);
+  Node* const this_arg = args.GetOptionalArgumentValue(1);
+
+  ThrowIfNotInstanceType(context, receiver, JS_SET_TYPE, kMethodName);
+
+  // Ensure that {callback} is actually callable.
+  Label callback_not_callable(this, Label::kDeferred);
+  GotoIf(TaggedIsSmi(callback), &callback_not_callable);
+  GotoIfNot(IsCallable(callback), &callback_not_callable);
+
+  VARIABLE(var_index, MachineType::PointerRepresentation(), IntPtrConstant(0));
+  VARIABLE(var_table, MachineRepresentation::kTagged,
+           LoadObjectField(receiver, JSSet::kTableOffset));
+  Label loop(this, {&var_index, &var_table}), done_loop(this);
+  Goto(&loop);
+  BIND(&loop);
+  {
+    // Transition {table} and {index} if there was any modification to
+    // the {receiver} while we're iterating.
+    Node* index = var_index.value();
+    Node* table = var_table.value();
+    std::tie(table, index) =
+        Transition<OrderedHashSet>(table, index, [](Node*, Node*) {});
+
+    // Read the next entry from the {table}, skipping holes.
+    Node* entry_key;
+    Node* entry_start_position;
+    std::tie(entry_key, entry_start_position, index) =
+        NextSkipHoles<OrderedHashSet>(table, index, &done_loop);
+
+    // Invoke the {callback} passing the {entry_key} (twice) and the {receiver}.
+    CallJS(CodeFactory::Call(isolate()), context, callback, this_arg, entry_key,
+           entry_key, receiver);
+
+    // Continue with the next entry.
+    var_index.Bind(index);
+    var_table.Bind(table);
+    Goto(&loop);
+  }
+
+  BIND(&done_loop);
+  args.PopAndReturn(UndefinedConstant());
+
+  BIND(&callback_not_callable);
+  {
+    CallRuntime(Runtime::kThrowCalledNonCallable, context, callback);
+    Unreachable();
+  }
+}
+
 TF_BUILTIN(SetPrototypeValues, CollectionsBuiltinsAssembler) {
   Node* const receiver = Parameter(Descriptor::kReceiver);
   Node* const context = Parameter(Descriptor::kContext);
@@ -952,19 +1086,16 @@ TF_BUILTIN(SetIteratorPrototypeNext, CollectionsBuiltinsAssembler) {
   BIND(&if_receiver_valid);
 
   // Check if the {receiver} is exhausted.
-  Label return_value(this), return_entry(this),
-      return_end(this, Label::kDeferred);
-  VARIABLE(var_value, MachineRepresentation::kTagged, UndefinedConstant());
   VARIABLE(var_done, MachineRepresentation::kTagged, TrueConstant());
-  // TODO(bmeurer): Don't stick undefined in here, but some canonical
-  // empty_table, to avoid this check.
-  GotoIf(IsUndefined(LoadObjectField(receiver, JSSetIterator::kTableOffset)),
-         &return_value);
+  VARIABLE(var_value, MachineRepresentation::kTagged, UndefinedConstant());
+  Label return_value(this, {&var_done, &var_value}), return_entry(this),
+      return_end(this, Label::kDeferred);
 
   // Transition the {receiver} table if necessary.
   Node* table;
   Node* index;
-  std::tie(table, index) = Transition<JSSetIterator, OrderedHashSet>(receiver);
+  std::tie(table, index) =
+      TransitionAndUpdate<JSSetIterator, OrderedHashSet>(receiver);
 
   // Read the next entry from the {table}, skipping holes.
   Node* entry_key;
@@ -997,8 +1128,7 @@ TF_BUILTIN(SetIteratorPrototypeNext, CollectionsBuiltinsAssembler) {
   BIND(&return_end);
   {
     StoreObjectFieldRoot(receiver, JSSetIterator::kTableOffset,
-                         Heap::kUndefinedValueRootIndex);
-    var_value.Bind(UndefinedConstant());
+                         Heap::kEmptyOrderedHashTableRootIndex);
     Goto(&return_value);
   }
 }

src/builtins/builtins-collections.cc

@@ -28,36 +28,6 @@ BUILTIN(MapClear) {
   return isolate->heap()->undefined_value();
 }
 
-BUILTIN(MapForEach) {
-  HandleScope scope(isolate);
-  const char* const kMethodName = "Map.prototype.forEach";
-  CHECK_RECEIVER(JSMap, map, kMethodName);
-
-  Handle<Object> callback_fn = args.atOrUndefined(isolate, 1);
-  if (!callback_fn->IsCallable()) {
-    THROW_NEW_ERROR_RETURN_FAILURE(
-        isolate,
-        NewTypeError(MessageTemplate::kCalledNonCallable, callback_fn));
-  }
-
-  Handle<Object> receiver = args.atOrUndefined(isolate, 2);
-  Handle<OrderedHashMap> table(OrderedHashMap::cast(map->table()), isolate);
-  Handle<JSMapIterator> iterator = isolate->factory()->NewJSMapIterator(
-      isolate->map_key_value_iterator_map(), table, 0);
-
-  while (iterator->HasMore()) {
-    Handle<Object> key(iterator->CurrentKey(), isolate);
-    Handle<Object> value(iterator->CurrentValue(), isolate);
-    Handle<Object> argv[] = {value, key, map};
-    RETURN_FAILURE_ON_EXCEPTION(
-        isolate,
-        Execution::Call(isolate, callback_fn, receiver, arraysize(argv), argv));
-    iterator->MoveNext();
-  }
-
-  return isolate->heap()->undefined_value();
-}
-
 BUILTIN(SetGetSize) {
   HandleScope scope(isolate);
   const char* const kMethodName = "get Set.prototype.size";
@@ -77,34 +47,5 @@ BUILTIN(SetClear) {
   return isolate->heap()->undefined_value();
 }
 
-BUILTIN(SetForEach) {
-  HandleScope scope(isolate);
-  const char* const kMethodName = "Set.prototype.forEach";
-  CHECK_RECEIVER(JSSet, set, kMethodName);
-
-  Handle<Object> callback_fn = args.atOrUndefined(isolate, 1);
-  if (!callback_fn->IsCallable()) {
-    THROW_NEW_ERROR_RETURN_FAILURE(
-        isolate,
-        NewTypeError(MessageTemplate::kCalledNonCallable, callback_fn));
-  }
-
-  Handle<Object> receiver = args.atOrUndefined(isolate, 2);
-  Handle<OrderedHashSet> table(OrderedHashSet::cast(set->table()), isolate);
-  Handle<JSSetIterator> iterator = isolate->factory()->NewJSSetIterator(
-      isolate->set_value_iterator_map(), table, 0);
-
-  while (iterator->HasMore()) {
-    Handle<Object> key(iterator->CurrentKey(), isolate);
-    Handle<Object> argv[] = {key, key, set};
-    RETURN_FAILURE_ON_EXCEPTION(
-        isolate,
-        Execution::Call(isolate, callback_fn, receiver, arraysize(argv), argv));
-    iterator->MoveNext();
-  }
-
-  return isolate->heap()->undefined_value();
-}
-
 }  // namespace internal
 }  // namespace v8

src/builtins/builtins-definitions.h

@@ -582,9 +582,10 @@ namespace internal {
   TFJ(MapHas, 1, kKey)                                                         \
   CPP(MapGetSize)                                                              \
   CPP(MapClear)                                                                \
-  CPP(MapForEach)                                                              \
   /* ES #sec-map.prototype.entries */                                          \
   TFJ(MapPrototypeEntries, 0)                                                  \
+  /* ES #sec-map.prototype.forEach */                                          \
+  TFJ(MapPrototypeForEach, SharedFunctionInfo::kDontAdaptArgumentsSentinel)    \
   /* ES #sec-map.prototype.keys */                                             \
   TFJ(MapPrototypeKeys, 0)                                                     \
   /* ES #sec-map.prototype.values */                                           \
@@ -873,9 +874,10 @@ namespace internal {
   TFJ(SetHas, 1, kKey)                                                         \
   CPP(SetGetSize)                                                              \
   CPP(SetClear)                                                                \
-  CPP(SetForEach)                                                              \
   /* ES #sec-set.prototype.entries */                                          \
   TFJ(SetPrototypeEntries, 0)                                                  \
+  /* ES #sec-set.prototype.foreach */                                          \
+  TFJ(SetPrototypeForEach, SharedFunctionInfo::kDontAdaptArgumentsSentinel)    \
   /* ES #sec-set.prototype.values */                                           \
   TFJ(SetPrototypeValues, 0)                                                   \
   /* ES #sec-%setiteratorprototype%.next */                                    \

src/heap/heap.cc

@@ -2789,6 +2789,16 @@ void Heap::CreateInitialObjects() {
   set_last_script_id(Smi::FromInt(v8::UnboundScript::kNoScriptId));
   set_next_template_serial_number(Smi::kZero);
 
+  // Allocate the empty OrderedHashTable.
+  Handle<FixedArray> empty_ordered_hash_table =
+      factory->NewFixedArray(OrderedHashMap::kHashTableStartIndex, TENURED);
+  empty_ordered_hash_table->set_map_no_write_barrier(
+      *factory->ordered_hash_table_map());
+  for (int i = 0; i < empty_ordered_hash_table->length(); ++i) {
+    empty_ordered_hash_table->set(i, Smi::kZero);
+  }
+  set_empty_ordered_hash_table(*empty_ordered_hash_table);
+
   // Allocate the empty script.
   Handle<Script> script = factory->NewScript(factory->empty_string());
   script->set_type(Script::TYPE_NATIVE);

src/heap/heap.h

@@ -167,6 +167,7 @@ using v8::MemoryPressureLevel;
   V(FixedArray, empty_sloppy_arguments_elements, EmptySloppyArgumentsElements) \
   V(SeededNumberDictionary, empty_slow_element_dictionary,                     \
     EmptySlowElementDictionary)                                                \
+  V(FixedArray, empty_ordered_hash_table, EmptyOrderedHashTable)               \
   V(PropertyCell, empty_property_cell, EmptyPropertyCell)                      \
   V(WeakCell, empty_weak_cell, EmptyWeakCell)                                  \
   V(InterceptorInfo, noop_interceptor_info, NoOpInterceptorInfo)               \

src/objects-debug.cc

@@ -954,8 +954,7 @@ void JSSetIterator::JSSetIteratorVerify() {
   CHECK(IsJSSetIterator());
   JSObjectVerify();
   VerifyHeapPointer(table());
-  Isolate* isolate = GetIsolate();
-  CHECK(table()->IsOrderedHashTable() || table()->IsUndefined(isolate));
+  CHECK(table()->IsOrderedHashTable());
   CHECK(index()->IsSmi());
 }
 
@@ -964,8 +963,7 @@ void JSMapIterator::JSMapIteratorVerify() {
   CHECK(IsJSMapIterator());
   JSObjectVerify();
   VerifyHeapPointer(table());
-  Isolate* isolate = GetIsolate();
-  CHECK(table()->IsOrderedHashTable() || table()->IsUndefined(isolate));
+  CHECK(table()->IsOrderedHashTable());
   CHECK(index()->IsSmi());
 }
 

src/objects.cc

@@ -18624,7 +18624,6 @@ template<class Derived, class TableType>
 bool OrderedHashTableIterator<Derived, TableType>::HasMore() {
   DisallowHeapAllocation no_allocation;
   Isolate* isolate = this->GetIsolate();
-  if (this->table()->IsUndefined(isolate)) return false;
 
   Transition();
 
@@ -18640,7 +18639,7 @@ bool OrderedHashTableIterator<Derived, TableType>::HasMore() {
 
   if (index < used_capacity) return true;
 
-  set_table(isolate->heap()->undefined_value());
+  set_table(isolate->heap()->empty_ordered_hash_table());
   return false;
 }