RFR: 8333133: Simplify QuickSort::sort

David Holmes dholmes at openjdk.org
Tue Jun 11 05:41:12 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

So .... IIUC the only code that would be affected by this change would be code that passes true, which could also have equivalent elements to sort, and which requires the sort order to always be the same regardless of the order the elements are found. I think only the archive related code cares about deterministic order, and package and module names should be unique, so this seems fine.

One pre-existing nit in a comment but otherwise looks good.

Thanks

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

> 41:   // We swap these three values into the right place in the array. This
> 42:   // means that this method not only returns the index of the pivot
> 43:   // element. It also alters the array so that:

Pre-existing nit: this should be one sentence: "... element, it also ..."

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

Marked as reviewed by dholmes (Reviewer).

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


More information about the shenandoah-dev mailing list