Optimize array clone for sealed, frozen objects

Improve micro-benchmark by ~5x
Before:
ApplySpreadLiteral
ApplySpreadLiteral-Numbers(Score): 279
SpreadCallSpreadLiteral
SpreadCallSpreadLiteral-Numbers(Score): 285

After:
ApplySpreadLiteral
ApplySpreadLiteral-Numbers(Score): 1074
SpreadCallSpreadLiteral
SpreadCallSpreadLiteral-Numbers(Score): 1009


Bug: v8:6831
Change-Id: Ifd676ca13d5b7e86afc1578636fdd4dc2733c474
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1628244
Commit-Queue: Z Nguyen-Huu <duongn@microsoft.com>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61862}

src/builtins/base.tq

@@ -434,6 +434,11 @@ transient type FastJSArrayForCopy extends FastJSArray;
 // A FastJSArray when the global ArrayIteratorProtector is not invalidated.
 transient type FastJSArrayWithNoCustomIteration extends FastJSArray;
 
+// A FastJSArrayForRead when the global ArrayIteratorProtector is not
+// invalidated.
+transient type FastJSArrayForReadWithNoCustomIteration extends
+    FastJSArrayForRead;
+
 type NoSharedNameSentinel extends Smi;
 
 type JSModuleNamespace extends JSObject;
@@ -1950,6 +1955,14 @@ Cast<FastJSArrayWithNoCustomIteration>(implicit context: Context)(
   return %RawDownCast<FastJSArrayWithNoCustomIteration>(o);
 }
 
+Cast<FastJSArrayForReadWithNoCustomIteration>(implicit context: Context)(
+    o: HeapObject): FastJSArrayForReadWithNoCustomIteration
+    labels CastError {
+  if (IsArrayIteratorProtectorCellInvalid()) goto CastError;
+  const a: FastJSArrayForRead = Cast<FastJSArrayForRead>(o) otherwise CastError;
+  return %RawDownCast<FastJSArrayForReadWithNoCustomIteration>(o);
+}
+
 Cast<JSReceiver>(implicit context: Context)(o: HeapObject): JSReceiver
     labels CastError {
   if (IsJSReceiver(o)) return %RawDownCast<JSReceiver>(o);
@@ -2856,6 +2869,12 @@ macro IsFastJSArrayWithNoCustomIteration(context: Context, o: Object): bool {
   return Is<FastJSArrayWithNoCustomIteration>(o);
 }
 
+@export
+macro IsFastJSArrayForReadWithNoCustomIteration(context: Context, o: Object):
+    bool {
+  return Is<FastJSArrayForReadWithNoCustomIteration>(o);
+}
+
 extern transitioning runtime
 CreateDataProperty(implicit context: Context)(JSReceiver, Object, Object);
 

src/builtins/builtins-array-gen.cc

@@ -553,8 +553,8 @@ TF_BUILTIN(CloneFastJSArray, ArrayBuiltinsAssembler) {
   TNode<JSArray> array = CAST(Parameter(Descriptor::kSource));
 
   CSA_ASSERT(this,
-             Word32Or(Word32BinaryNot(
-                          IsHoleyFastElementsKind(LoadElementsKind(array))),
+             Word32Or(Word32BinaryNot(IsHoleyFastElementsKindForRead(
+                          LoadElementsKind(array))),
                       Word32BinaryNot(IsNoElementsProtectorCellInvalid())));
 
   ParameterMode mode = OptimalParameterMode();
@@ -573,8 +573,8 @@ TF_BUILTIN(CloneFastJSArrayFillingHoles, ArrayBuiltinsAssembler) {
   TNode<JSArray> array = CAST(Parameter(Descriptor::kSource));
 
   CSA_ASSERT(this,
-             Word32Or(Word32BinaryNot(
-                          IsHoleyFastElementsKind(LoadElementsKind(array))),
+             Word32Or(Word32BinaryNot(IsHoleyFastElementsKindForRead(
+                          LoadElementsKind(array))),
                       Word32BinaryNot(IsNoElementsProtectorCellInvalid())));
 
   ParameterMode mode = OptimalParameterMode();

src/builtins/builtins-iterator-gen.cc

@@ -270,8 +270,10 @@ void IteratorBuiltinsAssembler::FastIterableToList(
     TVariable<Object>* var_result, Label* slow) {
   Label done(this), check_string(this), check_map(this), check_set(this);
 
-  GotoIfNot(IsFastJSArrayWithNoCustomIteration(context, iterable),
-            &check_string);
+  GotoIfNot(
+      Word32Or(IsFastJSArrayWithNoCustomIteration(context, iterable),
+               IsFastJSArrayForReadWithNoCustomIteration(context, iterable)),
+      &check_string);
 
   // Fast path for fast JSArray.
   *var_result =

src/codegen/code-stub-assembler.cc

@@ -4076,13 +4076,15 @@ Node* CodeStubAssembler::CloneFastJSArray(Node* context, Node* array,
   VARIABLE(var_new_elements, MachineRepresentation::kTagged);
   TVARIABLE(Int32T, var_elements_kind, LoadMapElementsKind(LoadMap(array)));
 
-  Label allocate_jsarray(this), holey_extract(this);
+  Label allocate_jsarray(this), holey_extract(this),
+      allocate_jsarray_main(this);
 
   bool need_conversion =
       convert_holes == HoleConversionMode::kConvertToUndefined;
   if (need_conversion) {
     // We need to take care of holes, if the array is of holey elements kind.
-    GotoIf(IsHoleyFastElementsKind(var_elements_kind.value()), &holey_extract);
+    GotoIf(IsHoleyFastElementsKindForRead(var_elements_kind.value()),
+           &holey_extract);
   }
 
   // Simple extraction that preserves holes.
@@ -4117,6 +4119,17 @@ Node* CodeStubAssembler::CloneFastJSArray(Node* context, Node* array,
   }
 
   BIND(&allocate_jsarray);
+
+  // Handle sealed, frozen elements kinds
+  CSA_ASSERT(this, IsElementsKindLessThanOrEqual(var_elements_kind.value(),
+                                                 LAST_FROZEN_ELEMENTS_KIND));
+  GotoIf(IsElementsKindLessThanOrEqual(var_elements_kind.value(),
+                                       LAST_FAST_ELEMENTS_KIND),
+         &allocate_jsarray_main);
+  var_elements_kind = Int32Constant(PACKED_ELEMENTS);
+  Goto(&allocate_jsarray_main);
+
+  BIND(&allocate_jsarray_main);
   // Use the cannonical map for the chosen elements kind.
   Node* native_context = LoadNativeContext(context);
   TNode<Map> array_map =
@@ -13396,6 +13409,19 @@ Node* CodeStubAssembler::IsHoleyFastElementsKind(Node* elements_kind) {
   return IsSetWord32(elements_kind, 1);
 }
 
+Node* CodeStubAssembler::IsHoleyFastElementsKindForRead(Node* elements_kind) {
+  CSA_ASSERT(this,
+             Uint32LessThanOrEqual(elements_kind,
+                                   Int32Constant(LAST_FROZEN_ELEMENTS_KIND)));
+
+  STATIC_ASSERT(HOLEY_SMI_ELEMENTS == (PACKED_SMI_ELEMENTS | 1));
+  STATIC_ASSERT(HOLEY_ELEMENTS == (PACKED_ELEMENTS | 1));
+  STATIC_ASSERT(HOLEY_DOUBLE_ELEMENTS == (PACKED_DOUBLE_ELEMENTS | 1));
+  STATIC_ASSERT(HOLEY_SEALED_ELEMENTS == (PACKED_SEALED_ELEMENTS | 1));
+  STATIC_ASSERT(HOLEY_FROZEN_ELEMENTS == (PACKED_FROZEN_ELEMENTS | 1));
+  return IsSetWord32(elements_kind, 1);
+}
+
 Node* CodeStubAssembler::IsElementsKindGreaterThan(
     Node* target_kind, ElementsKind reference_kind) {
   return Int32GreaterThan(target_kind, Int32Constant(reference_kind));

src/codegen/code-stub-assembler.h

@@ -2365,6 +2365,7 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
   Node* IsFastSmiOrTaggedElementsKind(Node* elements_kind);
   Node* IsFastSmiElementsKind(Node* elements_kind);
   Node* IsHoleyFastElementsKind(Node* elements_kind);
+  Node* IsHoleyFastElementsKindForRead(Node* elements_kind);
   Node* IsElementsKindGreaterThan(Node* target_kind,
                                   ElementsKind reference_kind);
   TNode<BoolT> IsElementsKindLessThanOrEqual(TNode<Int32T> target_kind,

test/mjsunit/object-freeze.js

@@ -777,3 +777,19 @@ assertEquals(arrSpread.length, arrLike.length);
 assertEquals(arrSpread[0], 'a');
 assertEquals(arrSpread[1], 'b');
 assertEquals(arrSpread[2], 'c');
+
+// Spread with holey
+function countArgs() {
+  return arguments.length;
+}
+var arr = [, 'b','c'];
+Object.freeze(arr);
+assertEquals(countArgs(...arr), 3);
+assertEquals(countArgs(...[...arr]), 3);
+assertEquals(countArgs.apply(this, [...arr]), 3);
+function checkUndefined() {
+  return arguments[0] === undefined;
+}
+assertTrue(checkUndefined(...arr));
+assertTrue(checkUndefined(...[...arr]));
+assertTrue(checkUndefined.apply(this, [...arr]));

test/mjsunit/object-prevent-extensions.js

@@ -441,3 +441,19 @@ assertEquals(arrSpread.length, arrLike.length);
 assertEquals(arrSpread[0], 'a');
 assertEquals(arrSpread[1], 'b');
 assertEquals(arrSpread[2], 'c');
+
+// Spread with holey
+function countArgs() {
+  return arguments.length;
+}
+var arr = [, 'b','c'];
+Object.preventExtensions(arr);
+assertEquals(countArgs(...arr), 3);
+assertEquals(countArgs(...[...arr]), 3);
+assertEquals(countArgs.apply(this, [...arr]), 3);
+function checkUndefined() {
+  return arguments[0] === undefined;
+}
+assertTrue(checkUndefined(...arr));
+assertTrue(checkUndefined(...[...arr]));
+assertTrue(checkUndefined.apply(this, [...arr]));

test/mjsunit/object-seal.js

@@ -749,3 +749,19 @@ assertEquals(arrSpread.length, arrLike.length);
 assertEquals(arrSpread[0], 'a');
 assertEquals(arrSpread[1], 'b');
 assertEquals(arrSpread[2], 'c');
+
+// Spread with holey
+function countArgs() {
+  return arguments.length;
+}
+var arr = [, 'b','c'];
+Object.seal(arr);
+assertEquals(countArgs(...arr), 3);
+assertEquals(countArgs(...[...arr]), 3);
+assertEquals(countArgs.apply(this, [...arr]), 3);
+function checkUndefined() {
+  return arguments[0] === undefined;
+}
+assertTrue(checkUndefined(...arr));
+assertTrue(checkUndefined(...[...arr]));
+assertTrue(checkUndefined.apply(this, [...arr]));