Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(138)

Issue 2693043009: [typedarrays] sort in C++ for undefined comparefn (Closed)

Created:
3 years, 10 months ago by Choongwoo Han
Modified:
3 years, 10 months ago
CC:
v8-reviews_googlegroups.com, Dan Ehrenberg
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[typedarrays] sort in C++ for no comparison function - If no comparison function is given for %TypedArray%.prototype.sort, sort the typedarray using std::sort in C++. This gets 20 times more benchmark score in Float64Array. - Move ValidateTypedArray in builtin-typedarray.cc to static inline method of JSTypedArray class. BUG=v8:5953 Review-Url: https://codereview.chromium.org/2693043009 Cr-Commit-Position: refs/heads/master@{#43427} Committed: https://chromium.googlesource.com/v8/v8/+/32ec5335a4e2e385ecf89984c8f5dcc667b5dfb7

Patch Set 1 #

Total comments: 1

Patch Set 2 : Split tests and codes #

Total comments: 3

Patch Set 3 : Move ValidateTypedArray to JSTypedArray:Validate #

Total comments: 1

Patch Set 4 : for win #

Patch Set 5 : static casting #

Patch Set 6 : [typedarrays] sort in C++ for no comparison function #

Unified diffs Side-by-side diffs Delta from patch set Stats (+73 lines, -33 lines) Patch
M src/builtins/builtins-typedarray.cc View 1 2 2 chunks +1 line, -13 lines 0 comments Download
M src/js/typedarray.js View 1 2 2 chunks +1 line, -20 lines 0 comments Download
M src/objects.h View 1 2 1 chunk +4 lines, -0 lines 0 comments Download
M src/objects-inl.h View 1 2 1 chunk +12 lines, -0 lines 0 comments Download
M src/runtime/runtime.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M src/runtime/runtime-typedarray.cc View 1 2 3 4 5 1 chunk +54 lines, -0 lines 0 comments Download

Messages

Total messages: 40 (26 generated)
Choongwoo Han
PTAL
3 years, 10 months ago (2017-02-15 01:16:03 UTC) #3
Benedikt Meurer
Thanks for the patch. Adding petermarshall@ and caitp@ for review. Please split the js-perf-test changes ...
3 years, 10 months ago (2017-02-15 05:08:04 UTC) #5
Choongwoo Han
On 2017/02/15 05:08:04, Benedikt Meurer wrote: > Thanks for the patch. Adding petermarshall@ and caitp@ ...
3 years, 10 months ago (2017-02-15 08:13:22 UTC) #7
caitp
https://codereview.chromium.org/2693043009/diff/20001/src/runtime/runtime-typedarray.cc File src/runtime/runtime-typedarray.cc (right): https://codereview.chromium.org/2693043009/diff/20001/src/runtime/runtime-typedarray.cc#newcode405 src/runtime/runtime-typedarray.cc:405: THROW_NEW_ERROR_RETURN_FAILURE( So, as Dan was saying, we probably want ...
3 years, 10 months ago (2017-02-15 12:24:21 UTC) #9
caitp
https://codereview.chromium.org/2693043009/diff/20001/src/runtime/runtime-typedarray.cc File src/runtime/runtime-typedarray.cc (right): https://codereview.chromium.org/2693043009/diff/20001/src/runtime/runtime-typedarray.cc#newcode405 src/runtime/runtime-typedarray.cc:405: THROW_NEW_ERROR_RETURN_FAILURE( On 2017/02/15 12:24:20, caitp wrote: > So, as ...
3 years, 10 months ago (2017-02-15 13:27:22 UTC) #10
Choongwoo Han
https://codereview.chromium.org/2693043009/diff/20001/src/runtime/runtime-typedarray.cc File src/runtime/runtime-typedarray.cc (right): https://codereview.chromium.org/2693043009/diff/20001/src/runtime/runtime-typedarray.cc#newcode405 src/runtime/runtime-typedarray.cc:405: THROW_NEW_ERROR_RETURN_FAILURE( On 2017/02/15 13:27:22, caitp wrote: > On 2017/02/15 ...
3 years, 10 months ago (2017-02-16 08:06:35 UTC) #12
Benedikt Meurer
Sorry for the delay. LGTM, thanks!
3 years, 10 months ago (2017-02-20 17:42:13 UTC) #13
rongjie
The compile error can be fixed with this polyfill: #ifdef V8_CC_MSVC namespace std { template ...
3 years, 10 months ago (2017-02-21 09:45:38 UTC) #18
caitp
https://codereview.chromium.org/2693043009/diff/40001/src/runtime/runtime-typedarray.cc File src/runtime/runtime-typedarray.cc (right): https://codereview.chromium.org/2693043009/diff/40001/src/runtime/runtime-typedarray.cc#newcode379 src/runtime/runtime-typedarray.cc:379: return std::signbit(x) ? true : false; \ What about ...
3 years, 10 months ago (2017-02-21 14:31:45 UTC) #19
caitp
3 years, 10 months ago (2017-02-21 14:31:47 UTC) #20
Choongwoo Han
okey. The compiler seems not so smart. I think I can just separate floating typed ...
3 years, 10 months ago (2017-02-22 11:10:48 UTC) #25
Choongwoo Han
simply static casting to double for std::isnan and std::signbit. I think their direct static casting ...
3 years, 10 months ago (2017-02-24 03:16:17 UTC) #34
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2693043009/100001
3 years, 10 months ago (2017-02-25 01:52:27 UTC) #37
commit-bot: I haz the power
3 years, 10 months ago (2017-02-25 02:55:02 UTC) #40
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/v8/v8/+/32ec5335a4e2e385ecf89984c8f5dcc667b...

Powered by Google App Engine
This is Rietveld 408576698