[
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:
us...@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)
