thrift-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (THRIFT-4592) JS: readI32 performance on large arrays is very poor in Chrome
Date Tue, 03 Jul 2018 18:21:00 GMT

    [ https://issues.apache.org/jira/browse/THRIFT-4592?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16531764#comment-16531764
] 

ASF GitHub Bot commented on THRIFT-4592:
----------------------------------------

arittr commented on issue #1569: THRIFT-4592: [JS] change readI32 to use Array.reverse/Array.pop
vs Array.shift
URL: https://github.com/apache/thrift/pull/1569#issuecomment-402249350
 
 
   @jeking3 thank you! appreciate it.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


> JS: readI32 performance on large arrays is very poor in Chrome
> --------------------------------------------------------------
>
>                 Key: THRIFT-4592
>                 URL: https://issues.apache.org/jira/browse/THRIFT-4592
>             Project: Thrift
>          Issue Type: Bug
>          Components: JavaScript - Library
>    Affects Versions: 0.11.0
>         Environment: Tested in Chrome 67.0.3396.87 and Safari 11.1.1 (13605.2.8) on
OSX.
>            Reporter: Drew Ritter
>            Assignee: James E. King III
>            Priority: Critical
>             Fix For: 0.12.0
>
>         Attachments: ThriftBenchmarker.js, readI32_pop.patch
>
>
> `readI32` ( [https://github.com/apache/thrift/blob/master/lib/js/src/thrift.js#L1311-L1341] ) performance
is extremely poor in Chrome when reading values from large arrays. This is due to the use
of Array.shift ( [https://github.com/apache/thrift/blob/master/lib/js/src/thrift.js#L1322] )
which seems to have performance issues in V8/Chrome.
>  
> As Chrome is a high market share browser I propose either changing this to use reverse/pop
or detect the appropriate strategy given array length. I think this is due to how V8 handles
large arrays and the heuristics used to determine whether data should be stored in a C-backed
array or hash map.
>  
> STR:
> 0) Download or copy the attached ThriftBenchmarker.js - this file has some helper functions
to build large arrays, as well as a copy/paste of the current `readI32` as well as one that
uses Array.reverse/Array.pop over Array.shift.
> 1) Run this test in Chrome - you can simply copy/paste the code into the Console.
> 2) Observe that the tests using Array.shift are quite slow in Chrome:
> {code:java}
> -- testing long arrays --
> shift test: 1084.566162109375ms
> pop test: 37.604248046875ms
> -- testing long arrays of strings --
> shift test: 5189.416015625ms
> pop test: 17.9189453125ms
> -- testing short arrays --
> shift test: 0.06005859375ms
> pop test: 0.016845703125ms
> -- testing short arrays of strings --
> shift test: 0.01611328125ms
> pop test: 0.01318359375ms{code}
> 3) Run this test in Safari (or a non-V8) browser.
> 4) Observe that the tests using Array.shift are far faster:
> {code:java}
> -- testing long arrays --
> shift test: 19.235ms
> pop test: 11.774ms
> - testing long arrays of strings --
> shift test: 13.087ms
> pop test: 12.805ms
> - testing short arrays --
> shift test: 0.049ms
> pop test: 0.054ms
> - testing short arrays of strings --
> shift test: 0.039ms
> pop test: 0.022ms{code}
> [^ThriftBenchmarker.js]



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Mime
View raw message