On Wed, 11 Oct 2023 20:31:05 GMT, Srinivas Vamsi Parasa <d...@openjdk.org> wrote:
>> Hi @vamsi-parasa, >> >> Both methods mixedInsertionSort and insertionSort are covered by intrinsics. >> But insertionSort is run on leftmnost (one) part only and on small ( < >> MAX_INSERTION_SORT_SIZE = 44) arrays. >> Do we actually need to use intrinsics for it? >> >> To have clear picture could you please run benchmarking to compare both >> cases: >> current implementation and implementation with Java insertionSort only? >> >> see changes `sort(int.class, a, Unsafe.ARRAY_INT_BASE_OFFSET, low, high, >> DualPivotQuicksort::insertionSort);` >> -> >> `insertionSort(a, low, high);` > >> To have clear picture could you please run benchmarking to compare both >> cases: current implementation and implementation with Java insertionSort >> only? >> >> see changes `sort(int.class, a, Unsafe.ARRAY_INT_BASE_OFFSET, low, high, >> DualPivotQuicksort::insertionSort);` -> `insertionSort(a, low, high);` > > @iaroslavski > Yes, the intrinsic is needed. > I remember, the same question was asked in the original PR and data was > provided in the original PR. > Even for a small say size=30, the AVX512 sort intrinsic is faster. > @vamsi-parasa, and no profit with @forceinline on insertionSort and > mixedInsertionSort. I suggest removing @forceinline from insertionSort and > mixedInsertionSort. @iaroslavski, Please see the change reverted. ------------- PR Comment: https://git.openjdk.org/jdk/pull/16124#issuecomment-1758522236
