TypedArray set method is slow when called with another typed array
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 2 Aug 2012 19:20:33 +0000 (19:20 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 2 Aug 2012 19:20:33 +0000 (19:20 +0000)
https://bugs.webkit.org/show_bug.cgi?id=92556

Patch by Arnaud Renevier <a.renevier@sisa.samsung.com> on 2012-08-02
Reviewed by Kenneth Russell.

PerformanceTests:

* Bindings/typed-array-set-from-typed.html: Added.

Source/WebCore:

When setting multiples values to a typed array from an array like
element, try to determine if the argument is a typed array. If so,
cast the argument to a typed array, and read each element with .item()
method. That avoid reading the value as a JSValue, and speedups set
method by approximatively 10x.

Introduce setWebGLArrayWithTypedArrayArgument template function which
checks if argument is a typed array. If so, it copies the data to
target typed array and returns true. Otherwise, it returns false.

Introduce copyTypedArrayBuffer template function which copies data
from a typed array to another one. This function is also used from
constructArrayBufferViewWithTypedArrayArgument.

* bindings/js/JSArrayBufferViewHelper.h:
(WebCore):
(WebCore::copyTypedArrayBuffer):
(WebCore::setWebGLArrayWithTypedArrayArgument):
(WebCore::setWebGLArrayHelper):
(WebCore::constructArrayBufferViewWithTypedArrayArgument):
* bindings/js/JSFloat32ArrayCustom.cpp:
(WebCore::JSFloat32Array::set):
* bindings/js/JSFloat64ArrayCustom.cpp:
(WebCore::JSFloat64Array::set):
* bindings/js/JSInt16ArrayCustom.cpp:
(WebCore::JSInt16Array::set):
* bindings/js/JSInt32ArrayCustom.cpp:
(WebCore::JSInt32Array::set):
* bindings/js/JSInt8ArrayCustom.cpp:
(WebCore::JSInt8Array::set):
* bindings/js/JSUint16ArrayCustom.cpp:
(WebCore::JSUint16Array::set):
* bindings/js/JSUint32ArrayCustom.cpp:
(WebCore::JSUint32Array::set):
* bindings/js/JSUint8ArrayCustom.cpp:
(WebCore::JSUint8Array::set):
* bindings/js/JSUint8ClampedArrayCustom.cpp:
(WebCore::JSUint8ClampedArray::set):

Source/WTF:

Add an checkInboundData function to TypedArrayBase to check if a
position will be not be out of bound or overflow from the typed array.

* wtf/TypedArrayBase.h:
(WTF::TypedArrayBase::checkInboundData):
(TypedArrayBase):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@124483 268f45cc-cd09-0410-ab3c-d52691b4dbfc

15 files changed:
PerformanceTests/Bindings/typed-array-set-from-typed.html [new file with mode: 0644]
PerformanceTests/ChangeLog
Source/WTF/ChangeLog
Source/WTF/wtf/TypedArrayBase.h
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSArrayBufferViewHelper.h
Source/WebCore/bindings/js/JSFloat32ArrayCustom.cpp
Source/WebCore/bindings/js/JSFloat64ArrayCustom.cpp
Source/WebCore/bindings/js/JSInt16ArrayCustom.cpp
Source/WebCore/bindings/js/JSInt32ArrayCustom.cpp
Source/WebCore/bindings/js/JSInt8ArrayCustom.cpp
Source/WebCore/bindings/js/JSUint16ArrayCustom.cpp
Source/WebCore/bindings/js/JSUint32ArrayCustom.cpp
Source/WebCore/bindings/js/JSUint8ArrayCustom.cpp
Source/WebCore/bindings/js/JSUint8ClampedArrayCustom.cpp

diff --git a/PerformanceTests/Bindings/typed-array-set-from-typed.html b/PerformanceTests/Bindings/typed-array-set-from-typed.html
new file mode 100644 (file)
index 0000000..74fc2c7
--- /dev/null
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<body>
+<script src="../resources/runner.js"></script>
+<script>
+
+var length = 10000000;
+var source = new Uint8Array(length);
+for (var i = 0; i < length; i++) {
+    source[i] = i;
+}
+var target = new Float64Array(length);
+
+PerfTestRunner.run(function() {
+    target.set(source);
+});
+</script>
+</body>
+
index 0eec750..9b903b9 100644 (file)
@@ -1,3 +1,12 @@
+2012-08-02  Arnaud Renevier  <a.renevier@sisa.samsung.com>
+
+        TypedArray set method is slow when called with another typed array
+        https://bugs.webkit.org/show_bug.cgi?id=92556
+
+        Reviewed by Kenneth Russell.
+
+        * Bindings/typed-array-set-from-typed.html: Added.
+
 2012-07-28  Ryosuke Niwa  <rniwa@webkit.org>
 
         run-perf-tests should generate a results page
index b3492bf..533f819 100644 (file)
@@ -1,3 +1,17 @@
+2012-08-02  Arnaud Renevier  <a.renevier@sisa.samsung.com>
+
+        TypedArray set method is slow when called with another typed array
+        https://bugs.webkit.org/show_bug.cgi?id=92556
+
+        Reviewed by Kenneth Russell.
+
+        Add an checkInboundData function to TypedArrayBase to check if a
+        position will be not be out of bound or overflow from the typed array.
+
+        * wtf/TypedArrayBase.h:
+        (WTF::TypedArrayBase::checkInboundData):
+        (TypedArrayBase):
+
 2012-08-01  Patrick Gansterer  <paroga@webkit.org>
 
         Replace WTF::getCurrentLocalTime() with GregorianDateTime::setToCurrentLocalTime()
index 185cf77..6d2a0f2 100644 (file)
@@ -73,6 +73,14 @@ class TypedArrayBase : public ArrayBufferView {
         return TypedArrayBase<T>::data()[index];
     }
 
+    bool checkInboundData(unsigned offset, unsigned pos) const
+    {
+        return (offset <= m_length
+            && offset + pos <= m_length
+            // check overflow
+            && offset + pos >= offset);
+    }
+
 protected:
     TypedArrayBase(PassRefPtr<ArrayBuffer> buffer, unsigned byteOffset, unsigned length)
         : ArrayBufferView(buffer, byteOffset)
index ee0c6eb..3b88278 100644 (file)
@@ -1,3 +1,49 @@
+2012-08-02  Arnaud Renevier  <a.renevier@sisa.samsung.com>
+
+        TypedArray set method is slow when called with another typed array
+        https://bugs.webkit.org/show_bug.cgi?id=92556
+
+        Reviewed by Kenneth Russell.
+
+        When setting multiples values to a typed array from an array like
+        element, try to determine if the argument is a typed array. If so,
+        cast the argument to a typed array, and read each element with .item()
+        method. That avoid reading the value as a JSValue, and speedups set
+        method by approximatively 10x.
+
+        Introduce setWebGLArrayWithTypedArrayArgument template function which
+        checks if argument is a typed array. If so, it copies the data to
+        target typed array and returns true. Otherwise, it returns false.
+
+        Introduce copyTypedArrayBuffer template function which copies data
+        from a typed array to another one. This function is also used from
+        constructArrayBufferViewWithTypedArrayArgument.
+
+        * bindings/js/JSArrayBufferViewHelper.h:
+        (WebCore):
+        (WebCore::copyTypedArrayBuffer):
+        (WebCore::setWebGLArrayWithTypedArrayArgument):
+        (WebCore::setWebGLArrayHelper):
+        (WebCore::constructArrayBufferViewWithTypedArrayArgument):
+        * bindings/js/JSFloat32ArrayCustom.cpp:
+        (WebCore::JSFloat32Array::set):
+        * bindings/js/JSFloat64ArrayCustom.cpp:
+        (WebCore::JSFloat64Array::set):
+        * bindings/js/JSInt16ArrayCustom.cpp:
+        (WebCore::JSInt16Array::set):
+        * bindings/js/JSInt32ArrayCustom.cpp:
+        (WebCore::JSInt32Array::set):
+        * bindings/js/JSInt8ArrayCustom.cpp:
+        (WebCore::JSInt8Array::set):
+        * bindings/js/JSUint16ArrayCustom.cpp:
+        (WebCore::JSUint16Array::set):
+        * bindings/js/JSUint32ArrayCustom.cpp:
+        (WebCore::JSUint32Array::set):
+        * bindings/js/JSUint8ArrayCustom.cpp:
+        (WebCore::JSUint8Array::set):
+        * bindings/js/JSUint8ClampedArrayCustom.cpp:
+        (WebCore::JSUint8ClampedArray::set):
+
 2012-08-02  Chris Fleizach  <cfleizach@apple.com>
 
         AXEnabled = false for AXIncrementors inside text fields
index 8c6f426..e028564 100644 (file)
 
 namespace WebCore {
 
-template <class T>
-JSC::JSValue setWebGLArrayHelper(JSC::ExecState* exec, T* impl, T* (*conversionFunc)(JSC::JSValue))
+template<class C, typename T>
+bool copyTypedArrayBuffer(C* target, ArrayBufferView* source, unsigned sourceLength, unsigned offset)
+{
+    ArrayBufferView::ViewType sourceType = source->getType();
+    ASSERT(sourceType != ArrayBufferView::TypeDataView);
+
+    if (target->getType() == sourceType) {
+        if (!target->set(static_cast<C*>(source), offset))
+            return false;
+        return true;
+    }
+
+    if (!target->checkInboundData(offset, sourceLength))
+        return false;
+
+    switch (sourceType) {
+    case ArrayBufferView::TypeInt8:
+        for (unsigned i = 0; i < sourceLength; ++i)
+            target->set(i + offset, (T)(static_cast<TypedArrayBase<signed char> *>(source)->item(i)));
+        break;
+    case ArrayBufferView::TypeUint8:
+    case ArrayBufferView::TypeUint8Clamped:
+        for (unsigned i = 0; i < sourceLength; ++i)
+            target->set(i + offset, (T)(static_cast<TypedArrayBase<unsigned char> *>(source)->item(i)));
+        break;
+    case ArrayBufferView::TypeInt16:
+        for (unsigned i = 0; i < sourceLength; ++i)
+            target->set(i + offset, (T)(static_cast<TypedArrayBase<signed short> *>(source)->item(i)));
+        break;
+    case ArrayBufferView::TypeUint16:
+        for (unsigned i = 0; i < sourceLength; ++i)
+            target->set(i + offset, (T)(static_cast<TypedArrayBase<unsigned short> *>(source)->item(i)));
+        break;
+    case ArrayBufferView::TypeInt32:
+        for (unsigned i = 0; i < sourceLength; ++i)
+            target->set(i + offset, (T)(static_cast<TypedArrayBase<int> *>(source)->item(i)));
+        break;
+    case ArrayBufferView::TypeUint32:
+        for (unsigned i = 0; i < sourceLength; ++i)
+            target->set(i + offset, (T)(static_cast<TypedArrayBase<unsigned int> *>(source)->item(i)));
+        break;
+    case ArrayBufferView::TypeFloat32:
+        for (unsigned i = 0; i < sourceLength; ++i)
+            target->set(i + offset, (T)(static_cast<TypedArrayBase<float> *>(source)->item(i)));
+        break;
+    case ArrayBufferView::TypeFloat64:
+        for (unsigned i = 0; i < sourceLength; ++i)
+            target->set(i + offset, (T)(static_cast<TypedArrayBase<double> *>(source)->item(i)));
+        break;
+    default:
+        break;
+    }
+
+    return true;
+}
+
+template <class C, typename T>
+bool setWebGLArrayWithTypedArrayArgument(JSC::ExecState* exec, C* impl)
+{
+    RefPtr<ArrayBufferView> array = toArrayBufferView(exec->argument(0));
+    if (!array)
+        return false;
+
+    ArrayBufferView::ViewType arrayType = array->getType();
+    if (arrayType == ArrayBufferView::TypeDataView)
+        return false;
+
+    unsigned offset = 0;
+    if (exec->argumentCount() == 2)
+        offset = exec->argument(1).toInt32(exec);
+
+    uint32_t length = asObject(exec->argument(0))->get(exec, JSC::Identifier(exec, "length")).toUInt32(exec);
+
+    if (!(copyTypedArrayBuffer<C, T>(impl, array.get(), length, offset)))
+        setDOMException(exec, INDEX_SIZE_ERR);
+
+    return true;
+}
+
+template<class C, typename T>
+JSC::JSValue setWebGLArrayHelper(JSC::ExecState* exec, C* impl)
 {
     if (exec->argumentCount() < 1)
         return JSC::throwSyntaxError(exec);
 
-    T* array = (*conversionFunc)(exec->argument(0));
-    if (array) {
-        // void set(in WebGL<T>Array array, [Optional] in unsigned long offset);
-        unsigned offset = 0;
-        if (exec->argumentCount() == 2)
-            offset = exec->argument(1).toInt32(exec);
-        if (!impl->set(array, offset))
-            setDOMException(exec, INDEX_SIZE_ERR);
-
+    if (setWebGLArrayWithTypedArrayArgument<C, T>(exec, impl))
+        // void set(in WebGL<>Array array, [Optional] in unsigned long offset);
         return JSC::jsUndefined();
-    }
 
     if (exec->argument(0).isObject()) {
         // void set(in sequence<long> array, [Optional] in unsigned long offset);
@@ -66,9 +137,7 @@ JSC::JSValue setWebGLArrayHelper(JSC::ExecState* exec, T* impl, T* (*conversionF
         if (exec->argumentCount() == 2)
             offset = exec->argument(1).toInt32(exec);
         uint32_t length = array->get(exec, JSC::Identifier(exec, "length")).toInt32(exec);
-        if (offset > impl->length()
-            || offset + length > impl->length()
-            || offset + length < offset)
+        if (!impl->checkInboundData(offset, length))
             setDOMException(exec, INDEX_SIZE_ERR);
         else {
             for (uint32_t i = 0; i < length; i++) {
@@ -105,49 +174,11 @@ PassRefPtr<C> constructArrayBufferViewWithTypedArrayArgument(JSC::ExecState* exe
         return array;
     }
 
-    if (array->getType() == sourceType) {
-        memcpy(array->baseAddress(), source->baseAddress(), length * sizeof(T));
+    if (!(copyTypedArrayBuffer<C, T>(array.get(), source.get(), length, 0))) {
+        setDOMException(exec, INDEX_SIZE_ERR);
         return array;
     }
 
-    switch (sourceType) {
-    case ArrayBufferView::TypeInt8:
-        for (unsigned i = 0; i < length; ++i)
-            array->set(i, (T)(static_cast<TypedArrayBase<signed char> *>(source.get())->item(i)));
-        break;
-    case ArrayBufferView::TypeUint8:
-    case ArrayBufferView::TypeUint8Clamped:
-        for (unsigned i = 0; i < length; ++i)
-            array->set(i, (T)(static_cast<TypedArrayBase<unsigned char> *>(source.get())->item(i)));
-        break;
-    case ArrayBufferView::TypeInt16:
-        for (unsigned i = 0; i < length; ++i)
-            array->set(i, (T)(static_cast<TypedArrayBase<signed short> *>(source.get())->item(i)));
-        break;
-    case ArrayBufferView::TypeUint16:
-        for (unsigned i = 0; i < length; ++i)
-            array->set(i, (T)(static_cast<TypedArrayBase<unsigned short> *>(source.get())->item(i)));
-        break;
-    case ArrayBufferView::TypeInt32:
-        for (unsigned i = 0; i < length; ++i)
-            array->set(i, (T)(static_cast<TypedArrayBase<int> *>(source.get())->item(i)));
-        break;
-    case ArrayBufferView::TypeUint32:
-        for (unsigned i = 0; i < length; ++i)
-            array->set(i, (T)(static_cast<TypedArrayBase<unsigned int> *>(source.get())->item(i)));
-        break;
-    case ArrayBufferView::TypeFloat32:
-        for (unsigned i = 0; i < length; ++i)
-            array->set(i, (T)(static_cast<TypedArrayBase<float> *>(source.get())->item(i)));
-        break;
-    case ArrayBufferView::TypeFloat64:
-        for (unsigned i = 0; i < length; ++i)
-            array->set(i, (T)(static_cast<TypedArrayBase<double> *>(source.get())->item(i)));
-        break;
-    default:
-        return 0;
-    }
-
     return array;
 }
 
index cade52e..d8f58c2 100644 (file)
@@ -45,7 +45,7 @@ JSC::JSValue toJS(JSC::ExecState* exec, JSDOMGlobalObject* globalObject, Float32
 
 JSC::JSValue JSFloat32Array::set(JSC::ExecState* exec)
 {
-    return setWebGLArrayHelper(exec, impl(), toFloat32Array);
+    return setWebGLArrayHelper<Float32Array, float>(exec, impl());
 }
 
 EncodedJSValue JSC_HOST_CALL JSFloat32ArrayConstructor::constructJSFloat32Array(ExecState* exec)
index 15619e3..e7642a0 100644 (file)
@@ -45,7 +45,7 @@ JSC::JSValue toJS(JSC::ExecState* exec, JSDOMGlobalObject* globalObject, Float64
 
 JSC::JSValue JSFloat64Array::set(JSC::ExecState* exec)
 {
-    return setWebGLArrayHelper(exec, impl(), toFloat64Array);
+    return setWebGLArrayHelper<Float64Array, double>(exec, impl());
 }
 
 EncodedJSValue JSC_HOST_CALL JSFloat64ArrayConstructor::constructJSFloat64Array(ExecState* exec)
index bc77a6d..01b3939 100644 (file)
@@ -45,7 +45,7 @@ JSC::JSValue toJS(JSC::ExecState* exec, JSDOMGlobalObject* globalObject, Int16Ar
 
 JSC::JSValue JSInt16Array::set(JSC::ExecState* exec)
 {
-    return setWebGLArrayHelper(exec, impl(), toInt16Array);
+    return setWebGLArrayHelper<Int16Array, short>(exec, impl());
 }
 
 EncodedJSValue JSC_HOST_CALL JSInt16ArrayConstructor::constructJSInt16Array(ExecState* exec)
index d85287f..eaa6e57 100644 (file)
@@ -45,7 +45,7 @@ JSC::JSValue toJS(JSC::ExecState* exec, JSDOMGlobalObject* globalObject, Int32Ar
 
 JSC::JSValue JSInt32Array::set(JSC::ExecState* exec)
 {
-    return setWebGLArrayHelper(exec, impl(), toInt32Array);
+    return setWebGLArrayHelper<Int32Array, int>(exec, impl());
 }
 
 EncodedJSValue JSC_HOST_CALL JSInt32ArrayConstructor::constructJSInt32Array(ExecState* exec)
index b6e1507..4f053e0 100644 (file)
@@ -46,7 +46,7 @@ JSC::JSValue toJS(JSC::ExecState* exec, JSDOMGlobalObject* globalObject, Int8Arr
 
 JSC::JSValue JSInt8Array::set(JSC::ExecState* exec)
 {
-    return setWebGLArrayHelper(exec, impl(), toInt8Array);
+    return setWebGLArrayHelper<Int8Array, signed char>(exec, impl());
 }
 
 EncodedJSValue JSC_HOST_CALL JSInt8ArrayConstructor::constructJSInt8Array(ExecState* exec)
index 47b4402..b9e52a2 100644 (file)
@@ -45,7 +45,7 @@ JSC::JSValue toJS(JSC::ExecState* exec, JSDOMGlobalObject* globalObject, Uint16A
 
 JSC::JSValue JSUint16Array::set(JSC::ExecState* exec)
 {
-    return setWebGLArrayHelper(exec, impl(), toUint16Array);
+    return setWebGLArrayHelper<Uint16Array, unsigned short>(exec, impl());
 }
 
 EncodedJSValue JSC_HOST_CALL JSUint16ArrayConstructor::constructJSUint16Array(ExecState* exec)
index aa8b826..70ef0ba 100644 (file)
@@ -45,7 +45,7 @@ JSC::JSValue toJS(JSC::ExecState* exec, JSDOMGlobalObject* globalObject, Uint32A
 
 JSC::JSValue JSUint32Array::set(JSC::ExecState* exec)
 {
-    return setWebGLArrayHelper(exec, impl(), toUint32Array);
+    return setWebGLArrayHelper<Uint32Array, unsigned int>(exec, impl());
 }
 
 EncodedJSValue JSC_HOST_CALL JSUint32ArrayConstructor::constructJSUint32Array(ExecState* exec)
index 6a4fa1d..db7a43a 100644 (file)
@@ -45,7 +45,7 @@ JSC::JSValue toJS(JSC::ExecState* exec, JSDOMGlobalObject* globalObject, Uint8Ar
 
 JSC::JSValue JSUint8Array::set(JSC::ExecState* exec)
 {
-    return setWebGLArrayHelper(exec, impl(), toUint8Array);
+    return setWebGLArrayHelper<Uint8Array, unsigned char>(exec, impl());
 }
 
 EncodedJSValue JSC_HOST_CALL JSUint8ArrayConstructor::constructJSUint8Array(ExecState* exec)
index 2d5ca78..c79bac0 100644 (file)
@@ -45,7 +45,7 @@ JSC::JSValue toJS(JSC::ExecState* exec, JSDOMGlobalObject* globalObject, Uint8Cl
 
 JSC::JSValue JSUint8ClampedArray::set(JSC::ExecState* exec)
 {
-    return setWebGLArrayHelper(exec, impl(), toUint8ClampedArray);
+    return setWebGLArrayHelper<Uint8ClampedArray, unsigned char>(exec, impl());
 }
 
 EncodedJSValue JSC_HOST_CALL JSUint8ClampedArrayConstructor::constructJSUint8ClampedArray(ExecState* exec)