Remove default_sorter
default_sorter is wasteful and arguably not a good default, it would be better to remove it in cpp-sort 2.0.0, here are a few reasons why:
- The goal of the library is to provide a variety of sorting algorithms and related tools to experiment; if one wants a default sorting algorithm which is good enough for most situations, then
std::sortor pdqsort are good defaults and don't require dragging a whole template-heavy library as a dependency. - The backing sorters used are either
pdq_sorterfor random-access-iterators orquick_sorterfor other iterators, which means that the complexity degrades from O(n log n) to O(n²) when the iterators are not random-access. It's quite a hard difference in some scenarios and not always acceptable for a default. - When the collection passed is a small fixed-size arrays, then
low_comparisons_sorteris used, which now feels kind of arbitrary (which specifically that one), but also super expensive! - Moreover when adapted with
stable_adapter,default_sortersimply usesmerge_sorter, which is a different algorithm with a different space complexity altogether. The difference might not matter much forstd::sort/std::stable_sortsince those are definitely presented as different algorithms, but this is cpp-sort and one should expectstable_adapterto actually apply its usual rules and not to switch to a wholly different algorithm. - The whole thing is wrapped in
self_sort_adapter, which is actually something you probably want to decide yourself, and not have as a default and not understand why the errors are sometimes coming from a different place. And I just noticed it but itsstable_adapterspecialization does not useself_sort_adapter, which both surprising and a bug. - One could argue that we need a default sorter for
cppsort::sortandcppsort::stable_sort, but I also plan to get rid of those in version 2.0.0 (Remove cppsort::sort and cppsort::stable_sort #167).
We are pretty much done for the design decision of killing default_sorter, however there are other down-to-earth reasons to remove it: the deep nesting of several sorters and adapters means that it is slow to compile and produces hard to read error messages, which is something we want to avoid (see #125 and #28) and especially so for a default sorter.
Here is a flame graph generated with Clang's -ftime-trace showing the time spent parsing some includes of the test every_sorter.cpp:
As can be seen, default_sorter takes as much time to parse as most of the other sorters combined, but it also produces a bunch of wasteful instantiations while most of its features will most likely end up unused - if not totally unwanted - in the common scenarios.
Basically it is a confusing & badly designed mess, and while some aspects could be changed/fixed, it will only waste resources and developer time, and does not quite fit the design of the library. It needs to go.
However it is a rather valuable tool for the test suite thanks to its deeply nested nature, so it might be worth preserving there under another name (without the stable_adapter specialization). A quick roadmap:
- Nuke
cppsort::default_sorter - Mark it as
[[deprecated]]in branch 1.x - Update the documentation accordingly
- Add an equivalent to the test suite under a different name
