Add fast path for spreading primitive strings.

This improves the performance on primitive strings of
IterableToListWithSymbolLookup, which implements the
CreateArrayFromIterable bytecode. The fast path is only
taken if the string iterator protector is valid (that is,
String.prototype[Symbol.iterator] and
String.prototype[Symbol.iterator]().next are untouched).

This brings spreading of primitive strings closer to the
performance of the string iterator optimizations.
(see https://docs.google.com/document/d/13z1fvRVpe_oEroplXEEX0a3WK94fhXorHjcOMsDmR-8/).

Bug: chromium:881273, v8:7980
Change-Id: Ic8d8619da2f2afcc9346203613a844f62653fd7a
Reviewed-on: https://chromium-review.googlesource.com/1243110
Commit-Queue: Hai Dang <dhai@google.com>
Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56329}

src/builtins/builtins-definitions.h

@@ -1126,6 +1126,7 @@ namespace internal {
   /* StringIterator */                                                         \
   /* ES6 #sec-%stringiteratorprototype%.next */                                \
   TFJ(StringIteratorPrototypeNext, 0, kReceiver)                               \
+  TFS(StringToList, kSource)                                                   \
                                                                                \
   /* Symbol */                                                                 \
   /* ES #sec-symbol-constructor */                                             \

src/builtins/builtins-iterator-gen.cc

@@ -5,6 +5,7 @@
 #include "src/builtins/builtins-iterator-gen.h"
 #include "src/builtins/growable-fixed-array-gen.h"
 
+#include "src/builtins/builtins-string-gen.h"
 #include "src/builtins/builtins-utils-gen.h"
 #include "src/builtins/builtins.h"
 #include "src/code-stub-assembler.h"
@@ -261,20 +262,37 @@ TF_BUILTIN(IterableToListMayPreserveHoles, IteratorBuiltinsAssembler) {
 }
 
 // This builtin loads the property Symbol.iterator as the iterator, and has a
-// fast path for fast arrays. In case of fast holey arrays, holes will be
-// converted to undefined to reflect iteration semantics. Note that replacement
-// by undefined is only correct when the NoElements protector is valid.
+// fast path for fast arrays and another one for strings. These fast paths will
+// only be taken if Symbol.iterator and the Iterator prototype are not modified
+// in a way that changes the original iteration behavior.
+// * In case of fast holey arrays, holes will be converted to undefined to
+// reflect iteration semantics. Note that replacement by undefined is only
+// correct when the NoElements protector is valid.
 TF_BUILTIN(IterableToListWithSymbolLookup, IteratorBuiltinsAssembler) {
   TNode<Context> context = CAST(Parameter(Descriptor::kContext));
   TNode<Object> iterable = CAST(Parameter(Descriptor::kIterable));
 
-  Label slow_path(this);
+  Label slow_path(this), check_string(this);
 
-  GotoIfNot(IsFastJSArrayWithNoCustomIteration(iterable, context), &slow_path);
-  // Here we are guaranteed that iterable is a fast JSArray with an original
-  // iterator.
+  GotoIfForceSlowPath(&slow_path);
+
+  GotoIfNot(IsFastJSArrayWithNoCustomIteration(iterable, context),
+            &check_string);
+
+  // Fast path for fast JSArray.
   TailCallBuiltin(Builtins::kCloneFastJSArrayFillingHoles, context, iterable);
 
+  BIND(&check_string);
+  {
+    StringBuiltinsAssembler string_assembler(state());
+    GotoIfNot(string_assembler.IsStringPrimitiveWithNoCustomIteration(iterable,
+                                                                      context),
+              &slow_path);
+
+    // Fast path for strings.
+    TailCallBuiltin(Builtins::kStringToList, context, iterable);
+  }
+
   BIND(&slow_path);
   {
     TNode<Object> iterator_fn = GetIteratorMethod(context, iterable);

src/builtins/builtins-string-gen.cc

@@ -2493,6 +2493,89 @@ TF_BUILTIN(StringIteratorPrototypeNext, StringBuiltinsAssembler) {
   }
 }
 
+TNode<BoolT> StringBuiltinsAssembler::IsStringPrimitiveWithNoCustomIteration(
+    TNode<Object> object, TNode<Context> context) {
+  Label if_false(this, Label::kDeferred), exit(this);
+  TVARIABLE(BoolT, var_result);
+
+  GotoIf(TaggedIsSmi(object), &if_false);
+  GotoIfNot(IsString(CAST(object)), &if_false);
+
+  // Check that the String iterator hasn't been modified in a way that would
+  // affect iteration.
+  Node* protector_cell = LoadRoot(RootIndex::kStringIteratorProtector);
+  DCHECK(isolate()->heap()->string_iterator_protector()->IsPropertyCell());
+  var_result =
+      WordEqual(LoadObjectField(protector_cell, PropertyCell::kValueOffset),
+                SmiConstant(Isolate::kProtectorValid));
+  Goto(&exit);
+
+  BIND(&if_false);
+  {
+    var_result = Int32FalseConstant();
+    Goto(&exit);
+  }
+
+  BIND(&exit);
+  return var_result.value();
+}
+
+TNode<JSArray> StringBuiltinsAssembler::StringToList(TNode<Context> context,
+                                                     TNode<String> string) {
+  CSA_ASSERT(this, IsStringPrimitiveWithNoCustomIteration(string, context));
+  const ElementsKind kind = PACKED_ELEMENTS;
+  TNode<IntPtrT> length = LoadStringLengthAsWord(string);
+
+  Node *array = nullptr, *elements = nullptr;
+  Node* array_map = LoadJSArrayElementsMap(kind, LoadNativeContext(context));
+  // Allocate both array and elements object, and initialize the JSArray.
+  std::tie(array, elements) = AllocateUninitializedJSArrayWithElements(
+      kind, array_map, SmiTag(length), nullptr, length);
+  // Put the array in a valid state because the loop below can trigger GC.
+  FillFixedArrayWithSmiZero(CAST(elements), length);
+
+  const int first_element_offset = FixedArray::kHeaderSize - kHeapObjectTag;
+  TNode<IntPtrT> first_to_element_offset =
+      ElementOffsetFromIndex(IntPtrConstant(0), kind, INTPTR_PARAMETERS, 0);
+  VARIABLE(
+      var_offset, MachineType::PointerRepresentation(),
+      IntPtrAdd(first_to_element_offset, IntPtrConstant(first_element_offset)));
+  TVARIABLE(IntPtrT, var_position, IntPtrConstant(0));
+  Label done(this), next_codepoint(this, {&var_position, &var_offset});
+
+  // TODO(dhai): refactor the loop construction for string to reuse.
+  Goto(&next_codepoint);
+
+  BIND(&next_codepoint);
+  {
+    // Loop condition.
+    GotoIfNot(IntPtrLessThan(var_position.value(), length), &done);
+    const UnicodeEncoding encoding = UnicodeEncoding::UTF16;
+    TNode<Int32T> ch =
+        LoadSurrogatePairAt(string, length, var_position.value(), encoding);
+    TNode<String> value = StringFromSingleCodePoint(ch, encoding);
+
+    Store(elements, var_offset.value(), value);
+
+    // Increment the position.
+    TNode<IntPtrT> ch_length = LoadStringLengthAsWord(value);
+    var_position = IntPtrAdd(var_position.value(), ch_length);
+    // Increment the array offset and continue the loop.
+    var_offset.Bind(
+        IntPtrAdd(var_offset.value(), IntPtrConstant(kPointerSize)));
+    Goto(&next_codepoint);
+  }
+
+  BIND(&done);
+  return UncheckedCast<JSArray>(array);
+}
+
+TF_BUILTIN(StringToList, StringBuiltinsAssembler) {
+  TNode<Context> context = CAST(Parameter(Descriptor::kContext));
+  TNode<String> string = CAST(Parameter(Descriptor::kSource));
+  Return(StringToList(context, string));
+}
+
 // -----------------------------------------------------------------------------
 // ES6 section B.2.3 Additional Properties of the String.prototype object
 

src/builtins/builtins-string-gen.h

@@ -23,8 +23,12 @@ class StringBuiltinsAssembler : public CodeStubAssembler {
                         Node* rhs, Node* rhs_instance_type,
                         TNode<IntPtrT> length, Label* if_equal,
                         Label* if_not_equal, Label* if_indirect);
+  TNode<BoolT> IsStringPrimitiveWithNoCustomIteration(TNode<Object> object,
+                                                      TNode<Context> context);
 
  protected:
+  TNode<JSArray> StringToList(TNode<Context> context, TNode<String> string);
+
   void StringEqual_Loop(Node* lhs, Node* lhs_instance_type,
                         MachineType lhs_type, Node* rhs,
                         Node* rhs_instance_type, MachineType rhs_type,

src/compiler/js-heap-broker.h

@@ -242,6 +242,7 @@ class ContextRef : public HeapObjectRef {
   V(JSFunction, promise_function)             \
   V(Map, fast_aliased_arguments_map)          \
   V(Map, initial_array_iterator_map)          \
+  V(Map, initial_string_iterator_map)         \
   V(Map, iterator_result_map)                 \
   V(Map, js_array_holey_double_elements_map)  \
   V(Map, js_array_holey_elements_map)         \
@@ -257,7 +258,6 @@ class ContextRef : public HeapObjectRef {
   V(Map, sloppy_arguments_map)                \
   V(Map, slow_object_with_null_prototype_map) \
   V(Map, strict_arguments_map)                \
-  V(Map, initial_string_iterator_map)         \
   V(ScriptContextTable, script_context_table)
 
 class NativeContextRef : public ContextRef {

src/heap/heap.h

@@ -150,6 +150,7 @@ using v8::MemoryPressureLevel;
   V(TypedArraySpeciesProtector)             \
   V(PromiseSpeciesProtector)                \
   V(StaleRegister)                          \
+  V(StringIteratorProtector)                \
   V(StringLengthProtector)                  \
   V(StringTableMap)                         \
   V(SymbolMap)                              \

tools/v8heapconst.py

@@ -377,6 +377,7 @@ KNOWN_OBJECTS = {
   ("OLD_SPACE", 0x02361): "StringLengthProtector",
   ("OLD_SPACE", 0x02371): "ArrayIteratorProtector",
   ("OLD_SPACE", 0x02399): "ArrayBufferNeuteringProtector",
+  ("OLD_SPACE", 0x02421): "StringIteratorProtector",
 }
 
 # List of known V8 Frame Markers.