RFR: 8185757: QuickSort array size should be size_t

[Thomas Schatzl]

Hi Kim,

On Wed, 2017-08-02 at 19:29 -0400, Kim Barrett wrote:
> Please review this change of the type of QuickSort::sort size
> parameter from int to size_t, and propogating this change throughout
> the QuickSort implementation (using size_t rather than int sizes and
> indices) and tests.
> 
> Since I was touching the QuickSort code anyway, I made a couple of
> additional changes.
> 
> - Re-ordered the internal template parameters, moving "idempotent" to
> the front and allowing the array element type and the comparator type
> to be deduced.
> 
> - Changed the handling of the result of calling the comparator, only
> requiring it to return negative, zero, or positive, rather than
> exactly -1, 0, or +1. This makes it consistent with the standard
> library function qsort.

Not sure if the change of the do-while to the for-loops improves
readability that much.
However, please put the closing brackets of these into extra lines
(quicksort.hpp:76,77) to avoid the casual reader to overlook them.

> Also updated a couple of callers:
> 
> - In G1CollectionSet::finalize_old_part, removed no longer needed
> cast of (size_t) size to int.

Yes :)

> 
> - In Method::sort_methods, removed unnecessary explicit template
> argument, allowing it to be deduced.  I didn't change the length from
> int to size_t here, because that had more fanout, and there are other
> issues around its type.  For example, it is being passed to other
> functions that expect a u2 value.

Fine with me. Please consider filing a CR for that.

> CR:
> https://bugs.openjdk.java.net/browse/JDK-8185757
> 
> Webrev:
> http://cr.openjdk.java.net/~kbarrett/8185757/hotspot.00/
> JPRT. In addition to unit testing, sorting gets exercised by G1 and
> by class file parsing (sort_methods).

One (potential) pre-existing issue with the test: it uses random() to
create tests - however this decreases reproducability...

Thanks,
  Thomas