[typedarrays] Reduce overheads of TA.p.set

Replace GetElement and SetElement to Get and Set, and use CopyElements,
which reduces 4x-13x overheads.

Bug: chromium:768775
Cq-Include-Trybots: master.tryserver.v8:v8_linux_noi18n_rel_ng
Change-Id: I58534b30c2035195c5f4b8f2c04e7c459bdbebaa
Reviewed-on: https://chromium-review.googlesource.com/720661Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48723}

src/builtins/builtins-typedarray.cc

@@ -279,12 +279,10 @@ BUILTIN(TypedArrayPrototypeReverse) {
 
 namespace {
 Object* TypedArrayCopyElements(Handle<JSTypedArray> target,
-                               Handle<JSReceiver> source, Object* length_obj) {
-  size_t length;
-  CHECK(TryNumberToSize(length_obj, &length));
-
+                               Handle<JSReceiver> source, uint32_t length,
+                               uint32_t offset) {
   ElementsAccessor* accessor = target->GetElementsAccessor();
-  return accessor->CopyElements(source, target, length);
+  return accessor->CopyElements(source, target, length, offset);
 }
 
 enum class TypedArraySetResultCodes {
@@ -299,30 +297,10 @@ enum class TypedArraySetResultCodes {
   NON_TYPED_ARRAY
 };
 
-MaybeHandle<Object> TypedArraySetFromArrayLike(Isolate* isolate,
-                                               Handle<JSTypedArray> target,
-                                               Handle<Object> source,
-                                               int source_length, int offset) {
-  DCHECK_GE(source_length, 0);
-  DCHECK_GE(offset, 0);
-
-  for (int i = 0; i < source_length; i++) {
-    Handle<Object> value;
-    ASSIGN_RETURN_ON_EXCEPTION(isolate, value,
-                               Object::GetElement(isolate, source, i), Object);
-    ASSIGN_RETURN_ON_EXCEPTION(isolate, value,
-                               Object::SetElement(isolate, target, offset + i,
-                                                  value, LanguageMode::kStrict),
-                               Object);
-  }
-
-  return target;
-}
-
 MaybeHandle<Object> TypedArraySetFromOverlapping(Isolate* isolate,
                                                  Handle<JSTypedArray> target,
                                                  Handle<JSTypedArray> source,
-                                                 int offset) {
+                                                 uint32_t offset) {
   DCHECK_GE(offset, 0);
 
   size_t sourceElementSize = source->element_size();
@@ -343,18 +321,14 @@ MaybeHandle<Object> TypedArraySetFromOverlapping(Isolate* isolate,
   uint32_t source_ptr = 0;
   CHECK(source->byte_offset()->ToUint32(&source_ptr));
 
+  ElementsAccessor* source_accessor = source->GetElementsAccessor();
+  ElementsAccessor* target_accessor = target->GetElementsAccessor();
+
   uint32_t left_index;
   for (left_index = 0; left_index < source_length && target_ptr <= source_ptr;
        left_index++) {
-    Handle<Object> value;
-    ASSIGN_RETURN_ON_EXCEPTION(isolate, value,
-                               Object::GetElement(isolate, source, left_index),
-                               Object);
-    ASSIGN_RETURN_ON_EXCEPTION(
-        isolate, value,
-        Object::SetElement(isolate, target, offset + left_index, value,
-                           LanguageMode::kStrict),
-        Object);
+    Handle<Object> value = source_accessor->Get(source, left_index);
+    target_accessor->Set(target, offset + left_index, *value);
 
     target_ptr += targetElementSize;
     source_ptr += sourceElementSize;
@@ -374,15 +348,8 @@ MaybeHandle<Object> TypedArraySetFromOverlapping(Isolate* isolate,
   DCHECK_GE(source_length, 1);
   for (right_index = source_length - 1;
        right_index > left_index && target_ptr >= source_ptr; right_index--) {
-    Handle<Object> value;
-    ASSIGN_RETURN_ON_EXCEPTION(isolate, value,
-                               Object::GetElement(isolate, source, right_index),
-                               Object);
-    ASSIGN_RETURN_ON_EXCEPTION(
-        isolate, value,
-        Object::SetElement(isolate, target, offset + right_index, value,
-                           LanguageMode::kStrict),
-        Object);
+    Handle<Object> value = source_accessor->Get(source, right_index);
+    target_accessor->Set(target, offset + right_index, *value);
 
     target_ptr -= targetElementSize;
     source_ptr -= sourceElementSize;
@@ -391,20 +358,11 @@ MaybeHandle<Object> TypedArraySetFromOverlapping(Isolate* isolate,
   std::vector<Handle<Object>> temp(right_index + 1 - left_index);
 
   for (uint32_t i = left_index; i <= right_index; i++) {
-    Handle<Object> value;
-    ASSIGN_RETURN_ON_EXCEPTION(isolate, value,
-                               Object::GetElement(isolate, source, i), Object);
-    temp[i - left_index] = value;
+    temp[i - left_index] = source_accessor->Get(source, i);
   }
 
   for (uint32_t i = left_index; i <= right_index; i++) {
-    Handle<Object> value;
-
-    ASSIGN_RETURN_ON_EXCEPTION(
-        isolate, value,
-        Object::SetElement(isolate, target, offset + i, temp[i - left_index],
-                           LanguageMode::kStrict),
-        Object);
+    target_accessor->Set(target, offset + i, *temp[i - left_index]);
   }
 
   return target;
@@ -476,6 +434,7 @@ BUILTIN(TypedArrayPrototypeSet) {
   Handle<Object> target = args.receiver();
   Handle<Object> obj = args.atOrUndefined(isolate, 1);
   Handle<Object> offset = args.atOrUndefined(isolate, 2);
+  const char* method = "%TypedArray%.prototype.set";
 
   if (offset->IsUndefined(isolate)) {
     offset = Handle<Object>(Smi::kZero, isolate);
@@ -498,13 +457,26 @@ BUILTIN(TypedArrayPrototypeSet) {
     THROW_NEW_ERROR_RETURN_FAILURE(
         isolate, NewTypeError(MessageTemplate::kNotTypedArray));
   }
-  auto int_offset = static_cast<int>(offset->Number());
 
+  Handle<JSTypedArray> target_array = Handle<JSTypedArray>::cast(target);
+  if (V8_UNLIKELY(target_array->WasNeutered())) {
+    const MessageTemplate::Template message =
+        MessageTemplate::kDetachedOperation;
+    Handle<String> operation =
+        isolate->factory()->NewStringFromAsciiChecked(method);
+    THROW_NEW_ERROR_RETURN_FAILURE(isolate, NewTypeError(message, operation));
+  }
+
+  uint32_t uint_offset;
+  CHECK(offset->ToUint32(&uint_offset));
+
+  // TODO(cwhan.tunz): Implement CopyElements for overlapping cases, and use
+  // TypedArrayCopyElements for all case instead of this result code based
+  // branches
   Handle<Smi> result_code;
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
       isolate, result_code,
-      TypedArraySetFastCases(isolate, Handle<JSTypedArray>::cast(target), obj,
-                             offset));
+      TypedArraySetFastCases(isolate, target_array, obj, offset));
 
   switch (static_cast<TypedArraySetResultCodes>(result_code->value())) {
     case TypedArraySetResultCodes::SAME_TYPE: {
@@ -512,23 +484,15 @@ BUILTIN(TypedArrayPrototypeSet) {
     }
     case TypedArraySetResultCodes::OVERLAPPING: {
       RETURN_FAILURE_ON_EXCEPTION(
-          isolate, TypedArraySetFromOverlapping(
-                       isolate, Handle<JSTypedArray>::cast(target),
-                       Handle<JSTypedArray>::cast(obj), int_offset));
+          isolate, TypedArraySetFromOverlapping(isolate, target_array,
+                                                Handle<JSTypedArray>::cast(obj),
+                                                uint_offset));
       break;
     }
     case TypedArraySetResultCodes::NONOVERLAPPING: {
-      if (int_offset == 0) {
-        TypedArrayCopyElements(Handle<JSTypedArray>::cast(target),
-                               Handle<JSTypedArray>::cast(obj),
-                               Handle<JSTypedArray>::cast(obj)->length());
-      } else {
-        RETURN_FAILURE_ON_EXCEPTION(
-            isolate,
-            TypedArraySetFromArrayLike(
-                isolate, Handle<JSTypedArray>::cast(target), obj,
-                Handle<JSTypedArray>::cast(obj)->length_value(), int_offset));
-      }
+      return TypedArrayCopyElements(
+          target_array, Handle<JSTypedArray>::cast(obj),
+          Handle<JSTypedArray>::cast(obj)->length_value(), uint_offset);
       break;
     }
     case TypedArraySetResultCodes::NON_TYPED_ARRAY: {
@@ -554,19 +518,16 @@ BUILTIN(TypedArrayPrototypeSet) {
       ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, len,
                                          Object::ToLength(isolate, len));
 
-      DCHECK_GE(int_offset, 0);
-      if (int_offset + len->Number() >
-          Handle<JSTypedArray>::cast(target)->length_value()) {
+      DCHECK_GE(uint_offset, 0);
+      if (uint_offset + len->Number() > target_array->length_value()) {
         THROW_NEW_ERROR_RETURN_FAILURE(
             isolate,
             NewRangeError(MessageTemplate::kTypedArraySetSourceTooLarge));
       }
       uint32_t int_l;
       CHECK(DoubleToUint32IfEqualToSelf(len->Number(), &int_l));
-      RETURN_FAILURE_ON_EXCEPTION(
-          isolate, TypedArraySetFromArrayLike(
-                       isolate, Handle<JSTypedArray>::cast(target), obj, int_l,
-                       int_offset));
+      return TypedArrayCopyElements(target_array, Handle<JSReceiver>::cast(obj),
+                                    int_l, uint_offset);
     } break;
   }
 

src/elements.cc

@@ -1020,13 +1020,14 @@ class ElementsAccessorBase : public ElementsAccessor {
   }
 
   Object* CopyElements(Handle<JSReceiver> source, Handle<JSObject> destination,
-                       size_t length) final {
-    return Subclass::CopyElementsHandleImpl(source, destination, length);
+                       size_t length, uint32_t offset) final {
+    return Subclass::CopyElementsHandleImpl(source, destination, length,
+                                            offset);
   }
 
   static Object* CopyElementsHandleImpl(Handle<JSReceiver> source,
                                         Handle<JSObject> destination,
-                                        size_t length) {
+                                        size_t length, uint32_t offset) {
     UNREACHABLE();
   }
 
@@ -3168,18 +3169,19 @@ class TypedElementsAccessor
 
   template <typename SourceTraits>
   static void CopyBetweenBackingStores(FixedTypedArrayBase* source,
-                                       BackingStore* dest, size_t length) {
+                                       BackingStore* dest, size_t length,
+                                       uint32_t offset) {
     FixedTypedArray<SourceTraits>* source_fta =
         FixedTypedArray<SourceTraits>::cast(source);
     for (uint32_t i = 0; i < length; i++) {
       typename SourceTraits::ElementType elem = source_fta->get_scalar(i);
-      dest->set(i, dest->from(elem));
+      dest->set(offset + i, dest->from(elem));
     }
   }
 
   static void CopyElementsHandleFromTypedArray(Handle<JSTypedArray> source,
                                                Handle<JSTypedArray> destination,
-                                               size_t length) {
+                                               size_t length, uint32_t offset) {
     // The source is a typed array, so we know we don't need to do ToNumber
     // side-effects, as the source elements will always be a number or
     // undefined.
@@ -3190,6 +3192,7 @@ class TypedElementsAccessor
     Handle<BackingStore> destination_elements(
         BackingStore::cast(destination->elements()));
 
+    DCHECK_LE(offset + source->length(), destination->length());
     DCHECK_GE(destination->length(), source->length());
     DCHECK(source->length()->IsSmi());
     DCHECK_EQ(Smi::FromInt(static_cast<int>(length)), source->length());
@@ -3219,15 +3222,16 @@ class TypedElementsAccessor
     // which have special conversion operations.
     if (same_type || (same_size && both_are_simple)) {
       size_t element_size = source->element_size();
-      std::memcpy(dest_data, source_data, length * element_size);
+      std::memcpy(dest_data + offset * element_size, source_data,
+                  length * element_size);
     } else {
       // We use scalar accessors below to avoid boxing/unboxing, so there are
       // no allocations.
       switch (source->GetElementsKind()) {
-#define TYPED_ARRAY_CASE(Type, type, TYPE, ctype, size)   \
-  case TYPE##_ELEMENTS:                                   \
-    CopyBetweenBackingStores<Type##ArrayTraits>(          \
-        *source_elements, *destination_elements, length); \
+#define TYPED_ARRAY_CASE(Type, type, TYPE, ctype, size)           \
+  case TYPE##_ELEMENTS:                                           \
+    CopyBetweenBackingStores<Type##ArrayTraits>(                  \
+        *source_elements, *destination_elements, length, offset); \
     break;
         TYPED_ARRAYS(TYPED_ARRAY_CASE)
         default:
@@ -3254,7 +3258,7 @@ class TypedElementsAccessor
 
   static bool TryCopyElementsHandleFastNumber(Handle<JSArray> source,
                                               Handle<JSTypedArray> destination,
-                                              size_t length) {
+                                              size_t length, uint32_t offset) {
     Isolate* isolate = source->GetIsolate();
     DisallowHeapAllocation no_gc;
     DisallowJavascriptExecution no_js(isolate);
@@ -3279,19 +3283,19 @@ class TypedElementsAccessor
         Object* elem = source_store->get(i);
         DCHECK(elem->IsSmi());
         int int_value = Smi::ToInt(elem);
-        dest->set(i, dest->from(int_value));
+        dest->set(offset + i, dest->from(int_value));
       }
       return true;
     } else if (kind == HOLEY_SMI_ELEMENTS) {
       FixedArray* source_store = FixedArray::cast(source->elements());
       for (uint32_t i = 0; i < length; i++) {
         if (source_store->is_the_hole(isolate, i)) {
-          dest->SetValue(i, undefined);
+          dest->SetValue(offset + i, undefined);
         } else {
           Object* elem = source_store->get(i);
           DCHECK(elem->IsSmi());
           int int_value = Smi::ToInt(elem);
-          dest->set(i, dest->from(int_value));
+          dest->set(offset + i, dest->from(int_value));
         }
       }
       return true;
@@ -3305,7 +3309,7 @@ class TypedElementsAccessor
         // Use the from_double conversion for this specific TypedArray type,
         // rather than relying on C++ to convert elem.
         double elem = source_store->get_scalar(i);
-        dest->set(i, dest->from(elem));
+        dest->set(offset + i, dest->from(elem));
       }
       return true;
     } else if (kind == HOLEY_DOUBLE_ELEMENTS) {
@@ -3313,10 +3317,10 @@ class TypedElementsAccessor
           FixedDoubleArray::cast(source->elements());
       for (uint32_t i = 0; i < length; i++) {
         if (source_store->is_the_hole(i)) {
-          dest->SetValue(i, undefined);
+          dest->SetValue(offset + i, undefined);
         } else {
           double elem = source_store->get_scalar(i);
-          dest->set(i, dest->from(elem));
+          dest->set(offset + i, dest->from(elem));
         }
       }
       return true;
@@ -3326,7 +3330,7 @@ class TypedElementsAccessor
 
   static Object* CopyElementsHandleSlow(Handle<JSReceiver> source,
                                         Handle<JSTypedArray> destination,
-                                        size_t length) {
+                                        size_t length, uint32_t offset) {
     Isolate* isolate = source->GetIsolate();
     Handle<BackingStore> destination_elements(
         BackingStore::cast(destination->elements()));
@@ -3336,13 +3340,21 @@ class TypedElementsAccessor
       ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, elem,
                                          Object::GetProperty(&it));
       ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, elem, Object::ToNumber(elem));
-      // We don't need to check for buffer neutering here, because the
-      // source cannot be a TypedArray.
+
+      if (V8_UNLIKELY(destination->WasNeutered())) {
+        const char* op = "set";
+        const MessageTemplate::Template message =
+            MessageTemplate::kDetachedOperation;
+        Handle<String> operation =
+            isolate->factory()->NewStringFromAsciiChecked(op);
+        THROW_NEW_ERROR_RETURN_FAILURE(isolate,
+                                       NewTypeError(message, operation));
+      }
       // The spec says we store the length, then get each element, so we don't
       // need to check changes to length.
-      destination_elements->SetValue(i, *elem);
+      destination_elements->SetValue(offset + i, *elem);
     }
-    return Smi::kZero;
+    return *isolate->factory()->undefined_value();
   }
 
   // This doesn't guarantee that the destination array will be completely
@@ -3350,28 +3362,31 @@ class TypedElementsAccessor
   // that is required.
   static Object* CopyElementsHandleImpl(Handle<JSReceiver> source,
                                         Handle<JSObject> destination,
-                                        size_t length) {
+                                        size_t length, uint32_t offset) {
+    Isolate* isolate = destination->GetIsolate();
     Handle<JSTypedArray> destination_ta =
         Handle<JSTypedArray>::cast(destination);
+    DCHECK_LE(offset + length, destination_ta->length_value());
 
     // All conversions from TypedArrays can be done without allocation.
     if (source->IsJSTypedArray()) {
       Handle<JSTypedArray> source_ta = Handle<JSTypedArray>::cast(source);
-      CopyElementsHandleFromTypedArray(source_ta, destination_ta, length);
-      return Smi::kZero;
+      CopyElementsHandleFromTypedArray(source_ta, destination_ta, length,
+                                       offset);
+      return *isolate->factory()->undefined_value();
     }
 
     // Fast cases for packed numbers kinds where we don't need to allocate.
     if (source->IsJSArray()) {
       Handle<JSArray> source_array = Handle<JSArray>::cast(source);
-      if (TryCopyElementsHandleFastNumber(source_array, destination_ta,
-                                          length)) {
-        return Smi::kZero;
+      if (TryCopyElementsHandleFastNumber(source_array, destination_ta, length,
+                                          offset)) {
+        return *isolate->factory()->undefined_value();
       }
     }
     // Final generic case that handles prototype chain lookups, getters, proxies
     // and observable side effects via valueOf, etc.
-    return CopyElementsHandleSlow(source, destination_ta, length);
+    return CopyElementsHandleSlow(source, destination_ta, length, offset);
   }
 };
 

src/elements.h

@@ -192,7 +192,8 @@ class ElementsAccessor {
                             Handle<FixedArrayBase> destination, int size) = 0;
 
   virtual Object* CopyElements(Handle<JSReceiver> source,
-                               Handle<JSObject> destination, size_t length) = 0;
+                               Handle<JSObject> destination, size_t length,
+                               uint32_t offset = 0) = 0;
 
   virtual Handle<FixedArray> CreateListFromArrayLike(Isolate* isolate,
                                                      Handle<JSObject> object,

test/mjsunit/es6/typedarray.js

@@ -25,6 +25,8 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+// Flags: --allow-natives-syntax
+
 // ArrayBuffer
 
 function TestByteLength(param, expectedByteLength) {
@@ -609,6 +611,20 @@ function TestTypedArraySet() {
   a101[0] = 42;
   b101.set(a101);
   assertArrayPrefix([42], b101);
+
+  // Detached array buffer when accessing a source element
+  var a111 = new Int8Array(100);
+  var evilarr = new Array(100);
+  var detached = false;
+  evilarr[1] = {
+    [Symbol.toPrimitive]() {
+      %ArrayBufferNeuter(a111.buffer);
+      detached = true;
+      return 1;
+    }
+  };
+  assertThrows(() => a111.set(evilarr), TypeError);
+  assertEquals(true, detached);
 }
 
 TestTypedArraySet();

test/test262/test262.status

@@ -109,11 +109,7 @@
   'built-ins/TypedArray/prototype/map/callbackfn-detachbuffer': [FAIL],
   'built-ins/TypedArray/prototype/reduce/callbackfn-detachbuffer': [FAIL],
   'built-ins/TypedArray/prototype/reduceRight/callbackfn-detachbuffer': [FAIL],
-  'built-ins/TypedArray/prototype/set/array-arg-targetbuffer-detached-on-get-src-value-throws': [FAIL],
-  'built-ins/TypedArray/prototype/set/array-arg-targetbuffer-detached-on-tointeger-offset-throws': [FAIL],
-  'built-ins/TypedArray/prototype/set/array-arg-targetbuffer-detached-throws': [FAIL],
   'built-ins/TypedArray/prototype/set/typedarray-arg-srcbuffer-detached-during-tointeger-offset-throws': [FAIL],
-  'built-ins/TypedArray/prototype/set/typedarray-arg-targetbuffer-detached-during-tointeger-offset-throws': [FAIL],
   'built-ins/TypedArray/prototype/slice/detached-buffer-custom-ctor-other-targettype': [FAIL],
   'built-ins/TypedArray/prototype/slice/detached-buffer-custom-ctor-same-targettype': [FAIL],
   'built-ins/TypedArray/prototype/slice/detached-buffer-get-ctor': [FAIL],