Title: [285971] trunk
Revision
285971
Author
sbar...@apple.com
Date
2021-11-17 18:47:45 -0800 (Wed, 17 Nov 2021)

Log Message

Run the memmove fast path in JSGenericTypedArrayView<Adaptor>::set when using a combination of Uint8 and Uint8Clamped
https://bugs.webkit.org/show_bug.cgi?id=233271
<rdar://85259288>

Reviewed by Yusuke Suzuki.

JSTests:

* microbenchmarks/typed-array-set-uint8-and-uint8clamped.js: Added.

Source/_javascript_Core:

We have a fast path for running memmove when both the thing being
copied from and the thing being copied to have the same typed array
type. However, when copying from a Uint8Array into a Uint8ClampedArray,
or vice versa, we were going down the slow path. In this case,
we can still take the fast path, since we're guaranteed that storing
into a Uint8ClampedArray from a Uint8Array will never actually need
to clamp values. And when storing from a Uint8ClampedArray into a
Uint8Array, the values can trivially be copied over.

This patch is a 100x speedup on the attached microbenchmark.

* runtime/JSGenericTypedArrayViewInlines.h:
(JSC::JSGenericTypedArrayView<Adaptor>::set):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (285970 => 285971)


--- trunk/JSTests/ChangeLog	2021-11-18 02:29:46 UTC (rev 285970)
+++ trunk/JSTests/ChangeLog	2021-11-18 02:47:45 UTC (rev 285971)
@@ -1,3 +1,13 @@
+2021-11-17  Saam Barati  <sbar...@apple.com>
+
+        Run the memmove fast path in JSGenericTypedArrayView<Adaptor>::set when using a combination of Uint8 and Uint8Clamped
+        https://bugs.webkit.org/show_bug.cgi?id=233271
+        <rdar://85259288>
+
+        Reviewed by Yusuke Suzuki.
+
+        * microbenchmarks/typed-array-set-uint8-and-uint8clamped.js: Added.
+
 2021-11-15  Yusuke Suzuki  <ysuz...@apple.com>
 
         [JSC] Use operation path when PutByVal child1 is not speculated as a Cell

Added: trunk/JSTests/microbenchmarks/typed-array-set-uint8-and-uint8clamped.js (0 => 285971)


--- trunk/JSTests/microbenchmarks/typed-array-set-uint8-and-uint8clamped.js	                        (rev 0)
+++ trunk/JSTests/microbenchmarks/typed-array-set-uint8-and-uint8clamped.js	2021-11-18 02:47:45 UTC (rev 285971)
@@ -0,0 +1,10 @@
+let x = new Uint8Array(10000);
+let y = new Uint8ClampedArray(10000);
+
+let start = Date.now();
+for (let i = 0; i < 10000; ++i) {
+    x.set(y);
+    y.set(x);
+}
+if (false)
+    print(Date.now() - start);

Modified: trunk/Source/_javascript_Core/ChangeLog (285970 => 285971)


--- trunk/Source/_javascript_Core/ChangeLog	2021-11-18 02:29:46 UTC (rev 285970)
+++ trunk/Source/_javascript_Core/ChangeLog	2021-11-18 02:47:45 UTC (rev 285971)
@@ -1,3 +1,25 @@
+2021-11-17  Saam Barati  <sbar...@apple.com>
+
+        Run the memmove fast path in JSGenericTypedArrayView<Adaptor>::set when using a combination of Uint8 and Uint8Clamped
+        https://bugs.webkit.org/show_bug.cgi?id=233271
+        <rdar://85259288>
+
+        Reviewed by Yusuke Suzuki.
+
+        We have a fast path for running memmove when both the thing being
+        copied from and the thing being copied to have the same typed array
+        type. However, when copying from a Uint8Array into a Uint8ClampedArray,
+        or vice versa, we were going down the slow path. In this case,
+        we can still take the fast path, since we're guaranteed that storing
+        into a Uint8ClampedArray from a Uint8Array will never actually need
+        to clamp values. And when storing from a Uint8ClampedArray into a
+        Uint8Array, the values can trivially be copied over.
+
+        This patch is a 100x speedup on the attached microbenchmark.
+
+        * runtime/JSGenericTypedArrayViewInlines.h:
+        (JSC::JSGenericTypedArrayView<Adaptor>::set):
+
 2021-11-17  Yusuke Suzuki  <ysuz...@apple.com>
 
         [JSC] Revise JSON.parse atomize policy

Modified: trunk/Source/_javascript_Core/runtime/JSGenericTypedArrayViewInlines.h (285970 => 285971)


--- trunk/Source/_javascript_Core/runtime/JSGenericTypedArrayViewInlines.h	2021-11-18 02:29:46 UTC (rev 285970)
+++ trunk/Source/_javascript_Core/runtime/JSGenericTypedArrayViewInlines.h	2021-11-18 02:47:45 UTC (rev 285971)
@@ -255,10 +255,8 @@
     VM& vm = globalObject->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
 
-    const ClassInfo* ci = object->classInfo(vm);
-    if (ci->typedArrayStorageType == Adaptor::typeValue) {
-        // The super fast case: we can just memmove since we're the same type.
-        JSGenericTypedArrayView* other = jsCast<JSGenericTypedArrayView*>(object);
+    auto memmoveFastPath = [&] (auto* 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));
@@ -267,8 +265,22 @@
         if (!success)
             return false;
 
+        RELEASE_ASSERT((std::is_same_v<decltype(typedVector()), decltype(other->typedVector())>));
         memmove(typedVector() + offset, other->typedVector() + objectOffset, length * elementSize);
         return true;
+    };
+
+    const ClassInfo* ci = object->classInfo(vm);
+    if (ci->typedArrayStorageType == Adaptor::typeValue)
+        return memmoveFastPath(jsCast<JSGenericTypedArrayView*>(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));
     }
     
     switch (ci->typedArrayStorageType) {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to