RFR: 8360023: Add an insertion sort implementation to Hotspot [v4]

Quan Anh Mai qamai at openjdk.org
Fri Jun 20 03:05:24 UTC 2025


On Thu, 19 Jun 2025 18:18:38 GMT, Cesar Soares Lucas <cslucas at openjdk.org> wrote:

>> Quan Anh Mai has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   give up on RandomIt
>
> src/hotspot/share/utilities/sort.hpp line 37:
> 
>> 35: public:
>> 36:   template <class T, class Compare>
>> 37:   static void sort(T* data, int size, Compare comp) {
> 
> `int size` to `size_t size` ? or at least `unsigned int`.

Hotspot container usually uses signed int for size. So I think `int` here is a sensible choice.

> src/hotspot/share/utilities/sort.hpp line 57:
> 
>> 55:         // backward)
>> 56:         T* prev = pos - 1;
>> 57:         if (comp(*prev, current_elem) <= 0) {
> 
> NIT: would be better to pass pointers here?

A `comp` usually receives references. Practically, it is almost the same as receiving pointers.

> test/hotspot/gtest/utilities/test_sort.cpp line 25:
> 
>> 23:  */
>> 24: 
>> 25: #include "runtime/os.hpp"
> 
> NIT: sort the imports?

I have cleaned up the unused import here. What do you mean by sorting the imports?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/25895#discussion_r2157947448
PR Review Comment: https://git.openjdk.org/jdk/pull/25895#discussion_r2157948428
PR Review Comment: https://git.openjdk.org/jdk/pull/25895#discussion_r2157948919


More information about the hotspot-dev mailing list