Skip to content
  • fpizlo@apple.com's avatar
    FloatTypedArrayAdaptor::toJSValue should almost certainly not use jsNumber()... · 1fb752ad
    fpizlo@apple.com authored
    FloatTypedArrayAdaptor::toJSValue should almost certainly not use jsNumber() since that attempts int conversions
    https://bugs.webkit.org/show_bug.cgi?id=120228
    
    Source/JavaScriptCore: 
    
    Reviewed by Oliver Hunt.
            
    It turns out that there were three problems:
            
    - Using jsNumber() meant that we were converting doubles to integers and then
      possibly back again whenever doing a set() between floating point arrays.
            
    - Slow-path accesses to double typed arrays were slower than necessary because
      of the to-int conversion attempt.
            
    - The use of JSValue as an intermediate for converting between differen types
      in typedArray.set() resulted in worse code than I had previously expected.
            
    This patch solves the problem by using template double-dispatch to ensure that
    that C++ compiler sees the simplest possible combination of casts between any
    combination of typed array types, while still preserving JS and typed array
    conversion semantics. Conversions are done as follows:
            
        SourceAdaptor::convertTo<TargetAdaptor>(value)
            
    Internally, convertTo() calls one of three possible methods on TargetAdaptor,
    with one method for each of int32_t, uint32_t, and double. This means that the
    C++ compiler will at worst see a widening cast to one of those types followed
    by a narrowing conversion (not necessarily a cast - may have clamping or the
    JS toInt32() function).
            
    This change doesn't just affect typedArray.set(); it also affects slow-path
    accesses to typed arrays as well. This patch also adds a bunch of new test
    coverage.
            
    This change is a ~50% speed-up on typedArray.set() involving floating point
    types.
    
    * GNUmakefile.list.am:
    * JavaScriptCore.vcxproj/JavaScriptCore.vcxproj:
    * JavaScriptCore.xcodeproj/project.pbxproj:
    * runtime/GenericTypedArrayView.h:
    (JSC::GenericTypedArrayView::set):
    * runtime/JSDataViewPrototype.cpp:
    (JSC::setData):
    * runtime/JSGenericTypedArrayView.h:
    (JSC::JSGenericTypedArrayView::setIndexQuicklyToDouble):
    (JSC::JSGenericTypedArrayView::setIndexQuickly):
    * runtime/JSGenericTypedArrayViewInlines.h:
    (JSC::::setWithSpecificType):
    (JSC::::set):
    * runtime/ToNativeFromValue.h: Added.
    (JSC::toNativeFromValue):
    * runtime/TypedArrayAdaptors.h:
    (JSC::IntegralTypedArrayAdaptor::toJSValue):
    (JSC::IntegralTypedArrayAdaptor::toDouble):
    (JSC::IntegralTypedArrayAdaptor::toNativeFromInt32):
    (JSC::IntegralTypedArrayAdaptor::toNativeFromUint32):
    (JSC::IntegralTypedArrayAdaptor::toNativeFromDouble):
    (JSC::IntegralTypedArrayAdaptor::convertTo):
    (JSC::FloatTypedArrayAdaptor::toJSValue):
    (JSC::FloatTypedArrayAdaptor::toDouble):
    (JSC::FloatTypedArrayAdaptor::toNativeFromInt32):
    (JSC::FloatTypedArrayAdaptor::toNativeFromUint32):
    (JSC::FloatTypedArrayAdaptor::toNativeFromDouble):
    (JSC::FloatTypedArrayAdaptor::convertTo):
    (JSC::Uint8ClampedAdaptor::toJSValue):
    (JSC::Uint8ClampedAdaptor::toDouble):
    (JSC::Uint8ClampedAdaptor::toNativeFromInt32):
    (JSC::Uint8ClampedAdaptor::toNativeFromUint32):
    (JSC::Uint8ClampedAdaptor::toNativeFromDouble):
    (JSC::Uint8ClampedAdaptor::convertTo):
    
    LayoutTests: 
    
    Reviewed by Oliver Hunt.
            
    Add coverage for three things:
            
    - Typed array accesses with corner-case values.
            
    - Typed array set() (i.e. copy) between arrays of different types.
            
    - Performance of typedArray.set() involving different types.
            
    This required some changes to our test harnesses, since they previously
    couldn't consistently do numerical array comparisons in a reliable way.
    
    * fast/js/regress/Float32Array-to-Float64Array-set-expected.txt: Added.
    * fast/js/regress/Float32Array-to-Float64Array-set.html: Added.
    * fast/js/regress/Float64Array-to-Int16Array-set-expected.txt: Added.
    * fast/js/regress/Float64Array-to-Int16Array-set.html: Added.
    * fast/js/regress/Int16Array-to-Int32Array-set-expected.txt: Added.
    * fast/js/regress/Int16Array-to-Int32Array-set.html: Added.
    * fast/js/regress/script-tests/Float32Array-to-Float64Array-set.js: Added.
    * fast/js/regress/script-tests/Float64Array-to-Int16Array-set.js: Added.
    * fast/js/regress/script-tests/Int16Array-to-Int32Array-set.js: Added.
    * fast/js/resources/js-test-pre.js:
    (areNumbersEqual):
    (areArraysEqual):
    (isResultCorrect):
    * fast/js/resources/standalone-pre.js:
    (areNumbersEqual):
    (areArraysEqual):
    (isTypedArray):
    (isResultCorrect):
    (stringify):
    (shouldBe):
    * fast/js/script-tests/typed-array-access.js: Added.
    (bitsToString):
    (bitsToValue):
    (valueToBits):
    (roundTrip):
    * fast/js/script-tests/typed-array-set-different-types.js: Added.
    (MyRandom):
    (.reference):
    (.usingConstruct):
    * fast/js/typed-array-access-expected.txt: Added.
    * fast/js/typed-array-access.html: Added.
    * fast/js/typed-array-set-different-types-expected.txt: Added.
    * fast/js/typed-array-set-different-types.html: Added.
    
    
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@154569 268f45cc-cd09-0410-ab3c-d52691b4dbfc
    1fb752ad