Patch to improve primitives Array.sort()

[Chan, Sunny]

An updated patch has been published to cr.openjdk via Paul: http://cr.openjdk.java.net/~psandoz/tmp/gs/sort/webrev.2/

Updates:
The testcase has been updated to clone the array
The redundant constant MAX_RUN_LENGTH has been removed.

From: Paul Sandoz [mailto:paul.sandoz at oracle.com]
Sent: 16 May 2015 00:13
To: Chan, Sunny [Tech]
Cc: O'Leary, Kristen [Tech]; 'Alan Bateman'; 'core-libs-dev at openjdk.java.net'; Rezaei, Mohammad A. [Tech]
Subject: Re: Patch to improve primitives Array.sort()


On May 15, 2015, at 11:48 AM, "Chan, Sunny" <Sunny.Chan at gs.com<mailto:Sunny.Chan at gs.com>> wrote:


I have provided Paul with an updated patch:

Here it is:

http://cr.openjdk.java.net/~psandoz/tmp/gs/sort/webrev.1/

In DualPivotQuicksort


  63     /**

  64      * The maximum length of run in merge sort.

  65      */

  66     private static final int MAX_RUN_LENGTH = 33;
You can remove this constant.



* The test has been updated using data provider and reduce as much repetition as possible.

Looking much better. Convention-wise if you don't have to deal with any check exceptions using a Supplier is more preferable to Callable. Up to you.


  56     @Test(dataProvider = "arrays")

  57     public void runTests(String testName, Callable<int[]> dataMethod) throws Exception

  58     {

  59         int[] array = dataMethod.call();

  60         this.sortAndAssert(array);

  61         this.sortAndAssert(this.floatCopyFromInt(array));

  62         this.sortAndAssert(this.doubleCopyFromInt(array));

  63         this.sortAndAssert(this.longCopyFromInt(array));

  64         this.sortAndAssert(this.shortCopyFromInt(array));

  65         this.sortAndAssert(this.charCopyFromInt(array));

At line 60 should you clone the array? otherwise, if i have looked correctly, that sorted result will be tested for other values.



* The GS copyright notice from the main JDK patch. However we retain it on our test cases as we developed ourselves. In our previous contributions where we provided new tests we have put our copyright along with oracle copyright and it was accepted (see: http://hg.openjdk.java.net/jdk9/dev/jdk/file/ed94f3e7ba6b/test/java/lang/instrument/DaemonThread/TestDaemonThread.java)

Ok. It was more query on my part. I believe it's ok for you to add copyright on both files if you wish.



* Alan has commented that there were older benchmark but searching through the archive I can see it mention "BentleyBasher" I cannot find the actual code that Vladimir used (thread: http://mail.openjdk.java.net/pipermail/core-libs-dev/2009-September/002633.html). Is there anywhere I can get hold of it?

I tried looking, but i cannot find any.

Paul.