Patch to improve primitives Array.sort() (original) (raw)

Chan, Sunny Sunny.Chan at gs.com
Tue May 19 08:48:56 UTC 2015


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.

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.

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

I tried looking, but i cannot find any.

Paul.



More information about the core-libs-dev mailing list