[libcxx-commits] [PATCH] D93233: [libc++] Replaces std::sort by Bitset sorting algorithm.

Louis Dionne via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Wed Oct 6 14:19:44 PDT 2021


ldionne commandeered this revision.
ldionne added a reviewer: minjaehwang.
ldionne added a comment.

I stumbled upon this again while looking at the review queue and realized I had never followed up. Please rest assured that the lack of response here is only a measure of how backed up we are with reviews and not how interested we are with this change. I also want to give special thanks to @Morwenn for responding to my request to review this and for the detailed explanation and interesting discussion that followed. For a non-sort-expert like me, that is essential in shedding light on the whole thing.

So my take away from the discussion is that Bitsetsort and pdqsort are comparable in efficiency, however Bitsetsort still suffers from O(n^2) worst case. Is that correct? If so, is it possible to modify Bitsetsort to avoid this O(n^2) worst case? That's a pretty bad offender in our conformance right now.

I'm going to commandeer this so I can rebase it onto `main` and apply a few changes necessary for libc++. I'm sorry this slept for so long. In particular, this patch as-is breaks the ABI because we stop exporting the internal `__sort_whatever` symbols from the dylib. We can't do that -- we're stuck with these symbols forever unfortunately. However, nothing prevents us from introducing new ones (even though for the time being I would try to avoid baking parts of the sort implementation in the dylib at all).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93233/new/

https://reviews.llvm.org/D93233



More information about the libcxx-commits mailing list