On Fri, 6 Oct 2023 18:45:59 GMT, Srinivas Vamsi Parasa <d...@openjdk.org> wrote:
>> My tier1-7 testing passed. Good. > >> My tier1-7 testing passed. Good. > > Thank you, Vladimir! Hi @vamsi-parasa, If DualPivotQuicksort.java is updated, can you improve `partitionDualPivot` and `partitionSinglePivot` methods a little bit also? We pass `int high` there and then assign `int end = high - 1;`, but later `high` is never used. It will be better to pass `end` instead of `high`, eliminate `int end = high - 1;` and don't introduce new variable. This is minor changes but sorting shows better performance. Please find my suggested code: 1. Replace high -> end, update javadoc: exclusive -> inclusive ` * @param end the index of the last element, inclusive, to be sorted` ` */` `int[] partition(A a, int low, int end, ...` ` * @param end the index of the last element, inclusive, to be sorted` ` */` `@IntrinsicCandidate` `@ForceInline` `private static <A> int[] partition(Class<?> elemType, A array, long offset, int low, int end, ...` ` return po.partition(array, low, end, pivotIndex1, pivotIndex2);` `}` `private static int[] partitionDualPivot(int[] a, int low, int end,` ` int lower = low;` ` int upper = end;` `private static int[] partitionSinglePivot(int[] a, int low, int end,` ` int lower = low;` ` int upper = end;` 2. Javadoc of `int[] partition(A a, int low, )` says 'to be sorted' for paramters a, low and high. It should be `to be partitioned`. Please correct. 3. The same for method `private static <A> int[] partition(Class<?> elemType,`, update javadoc. 4. Changes from Item 1. require correspondent changes in C++ sources, could please take care of it? 5. And changes from my previous comment: all 4 methods should have @ForceInline and all methods should be without @ForceInline. Am I right? I guess new PR should be opened for all mentioned changes. Could you please go ahead? I appreciate if you help to make sorting more accurate and better. ------------- PR Comment: https://git.openjdk.org/jdk/pull/14227#issuecomment-1751739561
