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.san...@oracle.com]
Sent: 16 May 2015 00:13
To: Chan, Sunny [Tech]
Cc: O'Leary, Kristen [Tech]; 'Alan Bateman'; 'core-libs-dev@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.c...@gs.com<mailto:sunny.c...@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.

Reply via email to