On Mon, 24 Jun 2024 21:01:14 GMT, Ludovic Henry <luhe...@openjdk.org> wrote:
>> Hi,
>> Can you help to review this patch?
>> Thanks!
>>
>> This is similar with previous JDK-8334396.
>> Added some tests.
>>
>> ### Test
>> <google-sheets-html-origin style="caret-color: rgb(0, 0, 0); color: rgb(0,
>> 0, 0); font-style: normal; font-variant-caps: normal; font-weight: 400;
>> letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px;
>> text-transform: none; white-space: normal; widows: auto; word-spacing: 0px;
>> -webkit-text-stroke-width: 0px; text-decoration: none;">
>> | Tests | Scores | Errors | Unit
>> -- | -- | -- | -- | --
>> Intrinsic, +zbb, +rvv | Characters.reverseBytes | 1654.535 | 69.36 | ns/op
>> | Shorts.reverseBytes | 1795.403 | 44.015 | ns/op
>> Intrinsic, +zbb, -rvv | Characters.reverseBytes | 1649.752 | 74.965 | ns/op
>> | Shorts.reverseBytes | 1798.637 | 49.52 | ns/op
>> Intrinsic, -zbb, +rvv | Characters.reverseBytes | 2279.588 | 44.222 | ns/op
>> | Shorts.reverseBytes | 2441.674 | 63.895 | ns/op
>> Intrinsic, -zbb, -rvv | Characters.reverseBytes | 2288.876 | 49.099 | ns/op
>> | Shorts.reverseBytes | 2454.454 | 94.004 | ns/op
>> No intrinsic | Characters.reverseBytes | 1629.722 | 23.656 | ns/op
>> | Shorts.reverseBytes | 2108.81 | 43.378 | ns/op
>>
>> </google-sheets-html-origin>
>
> test/micro/org/openjdk/bench/java/lang/Characters.java line 69:
>
>> 67:
>> 68: @Benchmark
>> 69: public void reverseBytes() {
>
> I'm not sure we want to be adding that benchmark to this class. Could you
> move to a different class that will test exclusively `reverseBytes` on
> `char[]`? You can then move the code from
> `test/micro/org/openjdk/bench/java/lang/Shorts.java` into that same class or
> a similarly named class for `short[]`.
Not sure if I understand you correctly.
The test is for reverseBytes of a char, not char[]. And it's quite similar as
existing test in for integer at:
https://github.com/openjdk/jdk/blob/master/test/micro/org/openjdk/bench/java/lang/Integers.java#L171
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19830#discussion_r1652215989
