Changeset 286639 in webkit

Timestamp:

Dec 7, 2021, 7:59:13 PM (5 years ago)

Author:

sbarati@apple.com

Message:

TypedArray prototype set should go down the fast path when using non clamped integer types of the same byte size
​https://bugs.webkit.org/show_bug.cgi?id=233905

Reviewed by Keith Miller.

JSTests:

• microbenchmarks/typed-array-prototype-set.js: Added.
• microbenchmarks/typed-array-prototype-set-order.js: Added.
• stress/typed-array-prototype-set.js: Added.

Source/JavaScriptCore:

We can use memmove in this scenario because the bitpattern of the
data between the signed and unsigned values will be the same.

This patch also fixes a bug where we were looking at the wrong
pointer when determining to do a forward or backwards loop in
our memmove. We were looking at the vector instead of vector+offset.

• runtime/JSGenericTypedArrayViewInlines.h:

(JSC::JSGenericTypedArrayView<Adaptor>::set):

LayoutTests:

• js/script-tests/typedarray-set-overlapping-elements-of-same-size.js:
• js/typedarray-set-overlapping-elements-of-same-size-expected.txt:

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/microbenchmarks/typed-array-prototype-set.js (added)
• JSTests/stress/typed-array-prototype-set-order.js (added)
• JSTests/stress/typed-array-prototype-set.js (added)
• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/js/script-tests/typedarray-set-overlapping-elements-of-same-size.js (modified) (1 diff)
• LayoutTests/js/typedarray-set-overlapping-elements-of-same-size-expected.txt (modified) (1 diff)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h (modified) (4 diffs)

trunk/JSTests/ChangeLog

@@ +1 @@
+2021-12-07  Saam Barati  <sbarati@apple.com>
+
+        TypedArray prototype set should go down the fast path when using non clamped integer types of the same byte size
+        https://bugs.webkit.org/show_bug.cgi?id=233905
+
+        Reviewed by Keith Miller.
+
+        * microbenchmarks/typed-array-prototype-set.js: Added.
+        * microbenchmarks/typed-array-prototype-set-order.js: Added.
+        * stress/typed-array-prototype-set.js: Added.
+
 2021-12-02  Yusuke Suzuki  <ysuzuki@apple.com>
 

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-12-07  Saam Barati  <sbarati@apple.com>
+
+        TypedArray prototype set should go down the fast path when using non clamped integer types of the same byte size
+        https://bugs.webkit.org/show_bug.cgi?id=233905
+
+        Reviewed by Keith Miller.
+
+        * js/script-tests/typedarray-set-overlapping-elements-of-same-size.js:
+        * js/typedarray-set-overlapping-elements-of-same-size-expected.txt:
+
 2021-12-07  Yoshiaki Jitsukawa  <yoshiaki.jitsukawa@sony.com>
 

trunk/LayoutTests/js/script-tests/typedarray-set-overlapping-elements-of-same-size.js

@@ -19 +19 @@
 }
 
-shouldBe("foo(10)", "[42,42,42,42,42,42,42,42,42,42,42]");
+shouldBe("foo(10)", "[42,42,43,44,45,46,47,48,49,50,51]");
 shouldBe("bar(10)", "[42,43,44,45,46,47,48,49,50,51,51]");
 

trunk/LayoutTests/js/typedarray-set-overlapping-elements-of-same-size-expected.txt

@@ -4 +4 @@
 
 
-PASS foo(10) is [42,42,42,42,42,42,42,42,42,42,42]
+PASS foo(10) is [42,42,43,44,45,46,47,48,49,50,51]
 PASS bar(10) is [42,43,44,45,46,47,48,49,50,51,51]
 PASS successfullyParsed is true

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2021-12-07  Saam Barati  <sbarati@apple.com>
+
+        TypedArray prototype set should go down the fast path when using non clamped integer types of the same byte size
+        https://bugs.webkit.org/show_bug.cgi?id=233905
+
+        Reviewed by Keith Miller.
+
+        We can use memmove in this scenario because the bitpattern of the
+        data between the signed and unsigned values will be the same.
+
+        This patch also fixes a bug where we were looking at the wrong
+        pointer when determining to do a forward or backwards loop in
+        our memmove. We were looking at the vector instead of vector+offset.
+
+        * runtime/JSGenericTypedArrayViewInlines.h:
+        (JSC::JSGenericTypedArrayView<Adaptor>::set):
+
 2021-12-07  Ross Kirsling  <ross.kirsling@sony.com>
 

trunk/Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h

@@ -217 +217 @@
     if (!hasArrayBuffer() || !other->hasArrayBuffer()
         || existingBuffer() != other->existingBuffer()
-        || (elementSize == otherElementSize && vector() <= other->vector())
+        || (elementSize == otherElementSize && (static_cast<void*>(typedVector() + offset) <= static_cast<void*>(other->typedVector() + otherOffset)))
         || type == CopyType::LeftToRight) {
         for (size_t i = 0; i < length; ++i) {
@@ -256 +256 @@
     auto scope = DECLARE_THROW_SCOPE(vm);
 
-    auto memmoveFastPath = [&] (auto* other) {
+    auto memmoveFastPath = [&] (JSArrayBufferView* other) {
         // The super fast case: we can just memmove since we're the same underlying storage type.
         length = std::min(length, other->length());
         
-        RELEASE_ASSERT(other->canAccessRangeQuickly(objectOffset, length));
         bool success = validateRange(globalObject, offset, length);
         EXCEPTION_ASSERT(!scope.exception() == success);
@@ -266 +265 @@
             return false;
 
-        RELEASE_ASSERT((std::is_same_v<decltype(typedVector()), decltype(other->typedVector())>));
-        memmove(typedVector() + offset, other->typedVector() + objectOffset, length * elementSize);
+        RELEASE_ASSERT(JSC::elementSize(Adaptor::typeValue) == JSC::elementSize(other->classInfo(vm)->typedArrayStorageType));
+        memmove(typedVector() + offset, bitwise_cast<typename Adaptor::Type*>(other->vector()) + objectOffset, length * elementSize);
         return true;
     };
@@ -273 +272 @@
     const ClassInfo* ci = object->classInfo(vm);
     if (ci->typedArrayStorageType == Adaptor::typeValue)
-        return memmoveFastPath(jsCast<JSGenericTypedArrayView*>(object));
+        return memmoveFastPath(jsCast<JSArrayBufferView*>(object));
 
     auto isSomeUint8 = [] (TypedArrayType type) {
         return type == TypedArrayType::TypeUint8 || type == TypedArrayType::TypeUint8Clamped;
     };
-    if (isSomeUint8(ci->typedArrayStorageType) && isSomeUint8(Adaptor::typeValue)) {
-        if (ci->typedArrayStorageType == TypedArrayType::TypeUint8)
-            return memmoveFastPath(jsCast<JSGenericTypedArrayView<Uint8Adaptor>*>(object));
-        return memmoveFastPath(jsCast<JSGenericTypedArrayView<Uint8ClampedAdaptor>*>(object));
-    }
-    
+
+    if (isSomeUint8(ci->typedArrayStorageType) && isSomeUint8(Adaptor::typeValue))
+        return memmoveFastPath(jsCast<JSArrayBufferView*>(object));
+
+    if (isInt(Adaptor::typeValue) && isInt(ci->typedArrayStorageType) && !isClamped(Adaptor::typeValue) && JSC::elementSize(Adaptor::typeValue) == JSC::elementSize(ci->typedArrayStorageType))
+        return memmoveFastPath(jsCast<JSArrayBufferView*>(object));
+
     switch (ci->typedArrayStorageType) {
     case TypeInt8: