Re: RFR: JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) [v9]

2021-11-28 Thread Laurent Bourgès
On Mon, 15 Nov 2021 16:20:07 GMT, iaroslavski  wrote:

>> Sorting:
>> 
>> - adopt radix sort for sequential and parallel sorts on 
>> int/long/float/double arrays (almost random and length > 6K)
>> - fix tryMergeRuns() to better handle case when the last run is a single 
>> element
>> - minor javadoc and comment changes
>> 
>> Testing:
>> - add new data inputs in tests for sorting
>> - add min/max/infinity values to float/double testing
>> - add tests for radix sort
>
> iaroslavski has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort)
>   
>   Updated comments for partitioning

As jdk18 Ramp Down 0 is approaching and jdk19 is being opened soon, maybe this 
PR review should be planned for jdk19 build 1 ?

-

PR: https://git.openjdk.java.net/jdk/pull/3938


Re: RFR: JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) [v9]

2021-11-18 Thread iaroslavski
On Mon, 15 Nov 2021 16:20:07 GMT, iaroslavski  wrote:

>> Sorting:
>> 
>> - adopt radix sort for sequential and parallel sorts on 
>> int/long/float/double arrays (almost random and length > 6K)
>> - fix tryMergeRuns() to better handle case when the last run is a single 
>> element
>> - minor javadoc and comment changes
>> 
>> Testing:
>> - add new data inputs in tests for sorting
>> - add min/max/infinity values to float/double testing
>> - add tests for radix sort
>
> iaroslavski has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort)
>   
>   Updated comments for partitioning

Hi all,

Not only Dual-Pivot Quicksort has been optimized, but I also improved test 
classes.
I measured coverage of these tests. It was shown that all methods, all branches 
are invoked.
So, we have full test coverage.

-

PR: https://git.openjdk.java.net/jdk/pull/3938


Re: RFR: JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) [v9]

2021-11-17 Thread Laurent Bourgès
On Mon, 15 Nov 2021 16:20:07 GMT, iaroslavski  wrote:

>> Sorting:
>> 
>> - adopt radix sort for sequential and parallel sorts on 
>> int/long/float/double arrays (almost random and length > 6K)
>> - fix tryMergeRuns() to better handle case when the last run is a single 
>> element
>> - minor javadoc and comment changes
>> 
>> Testing:
>> - add new data inputs in tests for sorting
>> - add min/max/infinity values to float/double testing
>> - add tests for radix sort
>
> iaroslavski has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort)
>   
>   Updated comments for partitioning

Please core-libs reviewers could you have a look before jdk18 rdp0 or not ?

-

PR: https://git.openjdk.java.net/jdk/pull/3938


Re: RFR: JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) [v9]

2021-11-15 Thread Laurent Bourgès
On Mon, 15 Nov 2021 16:20:07 GMT, iaroslavski  wrote:

>> Sorting:
>> 
>> - adopt radix sort for sequential and parallel sorts on 
>> int/long/float/double arrays (almost random and length > 6K)
>> - fix tryMergeRuns() to better handle case when the last run is a single 
>> element
>> - minor javadoc and comment changes
>> 
>> Testing:
>> - add new data inputs in tests for sorting
>> - add min/max/infinity values to float/double testing
>> - add tests for radix sort
>
> iaroslavski has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort)
>   
>   Updated comments for partitioning

As I am not an official openjdk reviewer, I can not approve, but I do support

-

PR: https://git.openjdk.java.net/jdk/pull/3938


Re: RFR: JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) [v9]

2021-11-15 Thread iaroslavski
On Mon, 15 Nov 2021 16:20:07 GMT, iaroslavski  wrote:

>> Sorting:
>> 
>> - adopt radix sort for sequential and parallel sorts on 
>> int/long/float/double arrays (almost random and length > 6K)
>> - fix tryMergeRuns() to better handle case when the last run is a single 
>> element
>> - minor javadoc and comment changes
>> 
>> Testing:
>> - add new data inputs in tests for sorting
>> - add min/max/infinity values to float/double testing
>> - add tests for radix sort
>
> iaroslavski has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort)
>   
>   Updated comments for partitioning

Hi reviewers!

@jhorstmann
@tarsa
@bourgesl
@richardstartin
@AlanBateman
@neliasso

I applied and tested all ideas/comments from Laurent and Alan,
the sorting sources (3 classes) are in final state.
Could you please review and approve the PR, if you don't have comments?

-

PR: https://git.openjdk.java.net/jdk/pull/3938


Re: RFR: JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) [v9]

2021-11-15 Thread iaroslavski
> Sorting:
> 
> - adopt radix sort for sequential and parallel sorts on int/long/float/double 
> arrays (almost random and length > 6K)
> - fix tryMergeRuns() to better handle case when the last run is a single 
> element
> - minor javadoc and comment changes
> 
> Testing:
> - add new data inputs in tests for sorting
> - add min/max/infinity values to float/double testing
> - add tests for radix sort

iaroslavski has updated the pull request incrementally with one additional 
commit since the last revision:

  JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort)
  
  Updated comments for partitioning

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3938/files
  - new: https://git.openjdk.java.net/jdk/pull/3938/files/e1b01cfb..4baa9a39

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3938&range=08
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3938&range=07-08

  Stats: 54 lines in 1 file changed: 0 ins; 0 del; 54 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3938.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3938/head:pull/3938

PR: https://git.openjdk.java.net/jdk/pull/3938