On Fri, 29 Sep 2023 07:41:08 GMT, Aleksey Shipilev <sh...@openjdk.org> wrote:
>> Michal Sobierski has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Better approach to parallelize sun/security/rsa/SignedObjectChain.java
>>
>> time make test TEST=jdk/sun/security/rsa/SignedObjectChain.java
>> TEST_VM_OPTS="-XX:+UseParallelGC -XX:TieredStopAtLevel=1"
>> before: 270.72s user 4.88s system 108% cpu 4:14.43 total
>> after: 375.46s user 4.59s system 539% cpu 1:10.41 total
>>
>> time make test TEST=jdk/sun/security/rsa/SignedObjectChain.java
>> TEST_VM_OPTS="-XX:+UseParallelGC"
>> before: 63.67s user 4.67s system 161% cpu 42.424 total
>> after: 67.31s user 4.48s system 417% cpu 17.183 total
>>
>> time make test TEST=jdk/sun/security/rsa/SignedObjectChain.java
>> TEST_VM_OPTS="-XX:+UseShenandoahGC -XX:TieredStopAtLevel=1"
>> before: 281.99s user 5.54s system 108% cpu 4:24.09 total
>> after: 413.51s user 5.08s system 613% cpu 1:08.25 total
>>
>> time make test TEST=jdk/sun/security/rsa/SignedObjectChain.java
>> TEST_VM_OPTS="-XX:+UseShenandoahGC"
>> before: 65.86s user 5.05s system 156% cpu 45.215 total
>> after: 83.25s user 4.82s system 469% cpu 18.741 total
>
> Or maybe @rhalade?
Looks good to me as well. Would be nice to apply the stylistic changes
suggested by @shipilev before integrating. Thanks!
> test/jdk/sun/security/rsa/SignedObjectChain.java line 58:
>
>> 56: boolean result =
>> Arrays.stream(tests).parallel().allMatch(Chain::runTest);
>> 57:
>> 58: if(result) {
>
> Suggestion:
>
> if (result) {
+1
> test/jdk/sun/security/rsa/SignedObjectChain.java line 65:
>
>> 63: }
>> 64: }
>> 65:
>
> Do we need a newline here?
+1, would be nice if this extra newline is removed.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/15860#issuecomment-1741280180
PR Review Comment: https://git.openjdk.org/jdk/pull/15860#discussion_r1341652737
PR Review Comment: https://git.openjdk.org/jdk/pull/15860#discussion_r1341653043
