[typedarrays] sort in C++ for undefined comparefn

[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

[Choongwoo Han, 2017-02-15 01:02:34]

Description was changed from

==========
[typedarrays] sort in C++ for undefined comparefn

- Add benchmark for sorting of Float64Array.
- If no compare function is given for %TypedArray%.prototype.sort,
sort the typedarray using std::sort in C++. This gets 20 times more
benchmark score in Float64Array.
- Rename existing typedarray.js to copywith.in in js-perf-test.

BUG=v8:5953
==========

to

==========
[typedarrays] sort in C++ for undefined comparefn

- Add benchmark for sorting of Float64Array.
- If no compare function is given for %TypedArray%.prototype.sort,
sort the typedarray using std::sort in C++. This gets 20 times more
benchmark score in Float64Array.
- Rename existing typedarray.js to copywith.in in js-perf-test.

BUG=v8:5953
==========

[Choongwoo Han, 2017-02-15 01:02:34]

cwhan.tunz@gmail.com changed reviewers:
+ bmeurer@chromium.org

[Choongwoo Han, 2017-02-15 01:16:03]

PTAL

[Benedikt Meurer, 2017-02-15 05:08:04]

bmeurer@chromium.org changed reviewers:
+ caitp@chromium.org, petermarshall@chromium.org

[Benedikt Meurer, 2017-02-15 05:08:04]

Thanks for the patch. Adding petermarshall@ and caitp@ for review.

Please split the js-perf-test changes into a separate CL and land those first,
so we actually track the improvements of this CL (and follow-up CLs).

https://codereview.chromium.org/2693043009/diff/1/src/js/typedarray.js
File src/js/typedarray.js (right):

https://codereview.chromium.org/2693043009/diff/1/src/js/typedarray.js#newcod...
src/js/typedarray.js:540: return %_TypedArraySortFast(this);
Since you call to the runtime anyways, just use %TypedArraySortFast here. The %_
notation is used for intrinsics, that can be provided by the various compilers,
and not necessarily end up as a runtime call.

[Choongwoo Han, 2017-02-15 08:12:08]

Description was changed from

==========
[typedarrays] sort in C++ for undefined comparefn

- Add benchmark for sorting of Float64Array.
- If no compare function is given for %TypedArray%.prototype.sort,
sort the typedarray using std::sort in C++. This gets 20 times more
benchmark score in Float64Array.
- Rename existing typedarray.js to copywith.in in js-perf-test.

BUG=v8:5953
==========

to

==========
[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.

BUG=v8:5953
==========

[Choongwoo Han, 2017-02-15 08:13:22]

On 2017/02/15 05:08:04, Benedikt Meurer wrote:
> Thanks for the patch. Adding petermarshall@ and caitp@ for review.
> 
> Please split the js-perf-test changes into a separate CL and land those first,
> so we actually track the improvements of this CL (and follow-up CLs).

Split done: https://codereview.chromium.org/2691423003/

> 
> https://codereview.chromium.org/2693043009/diff/1/src/js/typedarray.js
> File src/js/typedarray.js (right):
> 
>
https://codereview.chromium.org/2693043009/diff/1/src/js/typedarray.js#newcod...
> src/js/typedarray.js:540: return %_TypedArraySortFast(this);
> Since you call to the runtime anyways, just use %TypedArraySortFast here. The
%_
> notation is used for intrinsics, that can be provided by the various
compilers,
> and not necessarily end up as a runtime call.
Done

[caitp, 2017-02-15 12:24:20]

caitp@igalia.com changed reviewers:
+ caitp@igalia.com

[caitp, 2017-02-15 12:24:21]

https://codereview.chromium.org/2693043009/diff/20001/src/runtime/runtime-typ...
File src/runtime/runtime-typedarray.cc (right):

https://codereview.chromium.org/2693043009/diff/20001/src/runtime/runtime-typ...
src/runtime/runtime-typedarray.cc:405: THROW_NEW_ERROR_RETURN_FAILURE(
So, as Dan was saying, we probably want to jist early return if neutered, and
make all cases throw simultaneously in the same CL.

[caitp, 2017-02-15 13:27:22]

https://codereview.chromium.org/2693043009/diff/20001/src/runtime/runtime-typ...
File src/runtime/runtime-typedarray.cc (right):

https://codereview.chromium.org/2693043009/diff/20001/src/runtime/runtime-typ...
src/runtime/runtime-typedarray.cc:405: THROW_NEW_ERROR_RETURN_FAILURE(
On 2017/02/15 12:24:20, caitp wrote:
> So, as Dan was saying, we probably want to jist early return if neutered, and
> make all cases throw simultaneously in the same CL.

To add to this, maybe move ValidateTypedArray() in builtins-typedarray to a
static inline JSTypedArray method (`MaybeHandle<JSTypedArray>
JSTypedArray::Validate(isolate, Handle<Object>) ...` or something like that?)
and use that instead of duplicating the logic here. That will make it easier to
do the "throw in all algorithms simultaneously" later

[Choongwoo Han, 2017-02-16 08:04:51]

Description was changed from

==========
[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.

BUG=v8:5953
==========

to

==========
[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
==========

[Choongwoo Han, 2017-02-16 08:06:35]

https://codereview.chromium.org/2693043009/diff/20001/src/runtime/runtime-typ...
File src/runtime/runtime-typedarray.cc (right):

https://codereview.chromium.org/2693043009/diff/20001/src/runtime/runtime-typ...
src/runtime/runtime-typedarray.cc:405: THROW_NEW_ERROR_RETURN_FAILURE(
On 2017/02/15 13:27:22, caitp wrote:
> On 2017/02/15 12:24:20, caitp wrote:
> > So, as Dan was saying, we probably want to jist early return if neutered,
and
> > make all cases throw simultaneously in the same CL.
> 
> To add to this, maybe move ValidateTypedArray() in builtins-typedarray to a
> static inline JSTypedArray method (`MaybeHandle<JSTypedArray>
> JSTypedArray::Validate(isolate, Handle<Object>) ...` or something like that?)
> and use that instead of duplicating the logic here. That will make it easier
to
> do the "throw in all algorithms simultaneously" later

Done. I've moved it to static inline method of JSTypedArray.

[Benedikt Meurer, 2017-02-20 17:42:13]

Sorry for the delay. LGTM, thanks!

[Choongwoo Han, 2017-02-20 23:29:05]

The CQ bit was checked by cwhan.tunz@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-20 23:29:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-20 23:32:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-20 23:32:44]

Dry run: Try jobs failed on following builders:
  v8_win_compile_dbg on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_compile_dbg/builds/3...)
  v8_win_nosnap_shared_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)
  v8_win_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng/builds/23034)

[rongjie, 2017-02-21 09:45:38]

The compile error can be fixed with this polyfill:

#ifdef V8_CC_MSVC
namespace std {
template <typename T>
bool signbit(T _x) throw() {
	static_assert(std::is_integral<T>::value, "Must be integral");
	return signbit(static_cast<double>(_x));
}
}
#endif

See http://en.cppreference.com/w/cpp/numeric/math/signbit

[caitp, 2017-02-21 14:31:45]

https://codereview.chromium.org/2693043009/diff/40001/src/runtime/runtime-typ...
File src/runtime/runtime-typedarray.cc (right):

https://codereview.chromium.org/2693043009/diff/40001/src/runtime/runtime-typ...
src/runtime/runtime-typedarray.cc:379: return std::signbit(x) ? true : false;   
                  \
What about just

`return (std::is_integral<T>::value ? x < 0 : std::signbit(x)) ? true : false`

I think that should get past the msvc problem. You can make it easier to read by
using a variable instead of doing it all in one expression.

Should avoid needing to polyfill any libc++ methods

[Choongwoo Han, 2017-02-22 10:47:44]

The CQ bit was checked by cwhan.tunz@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-22 10:47:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-22 10:55:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 10:55:02]

Dry run: Try jobs failed on following builders:
  v8_win_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng/builds/23137)

[Choongwoo Han, 2017-02-22 11:10:48]

okey. The compiler seems not so smart. I think I can just separate floating
typed arrays and integer typed arrays (no callbacks for int typed arrays). but,
now i'm out of town so I'll check it out tomorrow.

[Choongwoo Han, 2017-02-23 14:35:49]

The CQ bit was checked by cwhan.tunz@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-23 14:35:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-23 14:39:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-23 14:39:09]

Dry run: Try jobs failed on following builders:
  v8_win_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng/builds/23190)

[Choongwoo Han, 2017-02-23 22:55:08]

The CQ bit was checked by cwhan.tunz@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-23 22:55:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-23 23:40:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-23 23:40:53]

Dry run: This issue passed the CQ dry run.

[Choongwoo Han, 2017-02-24 03:16:17]

simply static casting to double for std::isnan and std::signbit. I think their
direct static casting does not lose any information about NAN or signbit. seems
okey?

[Choongwoo Han, 2017-02-25 01:52:14]

The CQ bit was checked by cwhan.tunz@gmail.com

[Choongwoo Han, 2017-02-25 01:52:15]

The patchset sent to the CQ was uploaded after l-g-t-m from bmeurer@chromium.org
Link to the patchset: https://codereview.chromium.org/2693043009/#ps100001
(title: "[typedarrays] sort in C++ for no comparison function")

[commit-bot: I haz the power, 2017-02-25 01:52:27]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-25 02:54:54]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1487987534864450,
"parent_rev": "64a563c97f84c474a3f8b9a6b02c68ec1728cfee", "commit_rev":
"32ec5335a4e2e385ecf89984c8f5dcc667b5dfb7"}

[commit-bot: I haz the power, 2017-02-25 02:55:01]

Description was changed from

==========
[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
==========

to

==========
[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/+/32ec5335a4e2e385ecf89984c8f5dcc667b...
==========

[commit-bot: I haz the power, 2017-02-25 02:55:02]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/v8/v8/+/32ec5335a4e2e385ecf89984c8f5dcc667b...