TypedArray.from is 100+ x too slow
Reported by
maartenb...@gmail.com,
Sep 17
|
||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3497.81 Safari/537.36 Steps to reproduce the problem: See https://jsperf.com/comparing-typed-array-casting for a benchmark comparison. A good old look is about 100x faster than Float32Array.from(some_float_64_array). What is the expected behavior? It should be at least as fast. What went wrong? Float32Array.from is 100+ x slower than the manual loop Did this work before? N/A Chrome version: 69.0.3497.81 Channel: n/a OS Version: OS X 10.13.6 Flash Version:
,
Sep 17
Similar bugreport for firefox (15x slower): https://bugzilla.mozilla.org/show_bug.cgi?id=1491813
,
Sep 17
,
Sep 17
,
Sep 18
Able to reproduce the issue on Mac 10.13.3, Win-10 and Ubuntu 14.04 using chrome reported version #69.0.3497.81 and latest canary #71.0.3553.2. This is a non-regression issue as it is observed from M60 old builds. Hence, marking it as untriaged to get more inputs from dev team. Thanks...!!
,
Oct 22
how much is this anticipated?
,
Oct 23
Hi. I have a candidate fix for this issue. I'm having some firewall issues at the moment that are preventing me uploading the CL, hopefully I'll have those resolved tomorrow.
,
Oct 24
The other idiomatic way of doing this is just to use the typed array constructor. This is the same speed as the manual loop. The spec makes it easier for us because we don't have to interact with the iterator at all: https://tc39.github.io/ecma262/#sec-typedarray-typedarray I'll review Daniel Shelton's fix (https://chromium-review.googlesource.com/c/v8/v8/+/1297312) - we can probably add this fast path, just need to be careful to follow the spec exactly and make sure we don't introduce any security issues. var ar = new Float64Array(1e7); [ function() { new Float32Array(ar); }, function() { var N = ar.length; var ar32 = new Float32Array(N); for(var i = 0; i < N; i++) { ar32[i] = ar[i]; } } ].map(function(fn) { let start = Date.now(); fn(); console.log(Date.now() - start); });
,
Oct 26
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/c7c0e110f5414e7debc83cb396a53c7a0b56abd2 commit c7c0e110f5414e7debc83cb396a53c7a0b56abd2 Author: Peter Marshall <petermarshall@chromium.org> Date: Fri Oct 26 09:47:46 2018 [typedarray] Use fast path for Float32Array.from(float_64_array) and similar Currently, because the source float_64_array has an iterator, it hits the code in the "check_iterator" section of TypedArrayFrom which calls IterableToList. This builds a temporary PACKED_ELEMENTS array (and boxes all of the numeric values as HeapNumbers), then uses this as the source array. This patch checks if the source array is a TypedArray, and if the iterator is the built-in one (where we know the iterator's behaviour). If both are true then it bypasses the creation of this temporary array and uses the original TypedArray as the source. This allows it to take advantage of the existing fast code for copying one typed array to another. R=hablich@chromium.org, petermarshall@chromium.org Bug: chromium:884671 Change-Id: I19a944c9d6d5d07699c7dc3ad7196fc871200b62 Reviewed-on: https://chromium-review.googlesource.com/c/1297312 Reviewed-by: Peter Marshall <petermarshall@chromium.org> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org> Commit-Queue: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#57022} [modify] https://crrev.com/c7c0e110f5414e7debc83cb396a53c7a0b56abd2/AUTHORS [modify] https://crrev.com/c7c0e110f5414e7debc83cb396a53c7a0b56abd2/src/builtins/builtins-typed-array-gen.cc [add] https://crrev.com/c7c0e110f5414e7debc83cb396a53c7a0b56abd2/test/mjsunit/es6/typedarray-from-next-overridden.js [add] https://crrev.com/c7c0e110f5414e7debc83cb396a53c7a0b56abd2/test/mjsunit/es6/typedarray-from-nonfunction-iterator.js
,
Oct 26
Thanks for reporting this issue and thanks to Daniel for the fix! This should reach canary in the next few days
,
Oct 26
Great seeing this fixed. Thanks all! |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by woxxom@gmail.com
, Sep 17398 bytes
398 bytes View Download