RFR: 8333133: Simplify QuickSort::sort

Aleksey Shipilev shade at openjdk.org
Thu May 30 11:45:02 UTC 2024


On Wed, 29 May 2024 18:52:03 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

> The "idempotent" argument is removed from that function, with associated
> simplifications to the implementation. Callers are updated to remove that
> argument. Callers that were providing a false value are unaffected in their
> behavior.  The 3 callers that were providing a true value to request the
> associated feature are also unaffected (other than by being made faster),
> because the arrays involved don't contain any equivalent pairs.
> 
> There are also some miscellaneous cleanups, including using the swap utility
> and fixing some comments.
> 
> Testing: mach5 tier1-3

Looks reasonable.

src/hotspot/share/utilities/quickSort.hpp line 75:

> 73:     for ( ; true; ++left_index, --right_index) {
> 74:       for ( ; comparator(array[left_index], pivot_val) < 0; ++left_index) {
> 75:         assert(left_index < (length - 1), "reached end of partition");

Let me see if I understand this change. It makes assert stronger: we do not accept `left_index == length - 1` anymore. I guess that would mean the pivot is at the last element? Which makes the partition is empty, which cannot happen?

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

Marked as reviewed by shade (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19464#pullrequestreview-2088033775
PR Review Comment: https://git.openjdk.org/jdk/pull/19464#discussion_r1620546735


More information about the shenandoah-dev mailing list