On May 15, 2015, at 11:48 AM, "Chan, Sunny" <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.