Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The random-access version of quick_merge_sort runs in O(n²) #179

Closed
Morwenn opened this issue Dec 18, 2020 · 2 comments
Closed

The random-access version of quick_merge_sort runs in O(n²) #179

Morwenn opened this issue Dec 18, 2020 · 2 comments
Labels
Milestone

Comments

@Morwenn
Copy link
Owner

Morwenn commented Dec 18, 2020

I ran a quicksort killer like the one described in #178 with the following algorithms from the library - the ones that have a quicksort-like structure:

  • quick_sort
  • pdq_sort
  • quick_merge_sort (both the bidirectional rand random-access iterators versions)

Here are the results:

Benchmark of the number of comparisons performed against a quicksort killer input

It is quite easy to see the difference between the O(n log n) and the O(n²) algorithms there. And it clearly shows that the random-access version of quick_merge_sort runs in O(n²). The culprit is the random-access nth_element, which also means that the libc++ implementation of std::nth_element is quadratic.

@Morwenn Morwenn added the bug label Dec 18, 2020
@Morwenn
Copy link
Owner Author

Morwenn commented Jan 23, 2021

I tried to change the implementation of nth_element for random-access iterators to miniselect's implementation of Andrei Alexandrescu's QuickselectAdaptive selection algorithm. It is very slightly slower than the libc++ algorithm on average, but correctly handles the killer input without going quadratic:

Benchmark of the number of comparisons performed against a quicksort killer input

Next step is to steal the miniselect implementation and adapt it as needed.

@Morwenn Morwenn added this to the 1.9.0 milestone Jan 23, 2021
Morwenn added a commit that referenced this issue Jan 24, 2021
This makes quick_merge_sort O(n log n) instead of O(n²) like it used to
be (the libc++ implementation of std::nth_element used until then was
quadratic).

The new implementation was taken from Danila Kutenin's miniselect
library.
@Morwenn Morwenn closed this as completed Jan 24, 2021
@Morwenn
Copy link
Owner Author

Morwenn commented Dec 16, 2021

Related: llvm/llvm-project#52747

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant