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

Is extra::sort::tim_sort stable? #9676

Closed
SimonSapin opened this issue Oct 2, 2013 · 7 comments
Closed

Is extra::sort::tim_sort stable? #9676

SimonSapin opened this issue Oct 2, 2013 · 7 comments

Comments

@SimonSapin
Copy link
Contributor

Is extra::sort::tim_sort the same algorithm as Python’s Timsort? The documentation do not say much. In particular, can I rely on this sort being stable? (servo/servo#1001 is waiting on this.)

I can provide a documentation patch this is confirmed.

@SimonSapin
Copy link
Contributor Author

@14427, you seem to be the one who added this. Can you confirm?

@pnkfelix
Copy link
Member

pnkfelix commented Oct 2, 2013

@SimonSapin according to https://en.wikipedia.org/wiki/Timsort , the method is not stable (unless elements in a run "are present in strictly descending order", which I simplify to just "it is not stable").

but then again, http://c2.com/cgi/wiki?TimSort claims that it is stable. So I guess one might have to actually read the code. :(

@SimonSapin
Copy link
Contributor Author

I’m fairly sure that TimSort as found in Python is stable. In the Wikipedia sentence you quoted, "this method" refers not to TimSort in general but for the step of finding sub-sequence that are in reverse order. To preserve stability, this method is only used when the sub-sequence is in strictly descending order.

Although it very probably does, I just want to double-check that this code implements the same algorithm.

@thestinger
Copy link
Contributor

I'm planning on writing a dead simple merge sort for libstd (dropping down to insertion sort for less than ~32 elements) and dropping the extra::sort module completely. A heap sort will be available through the priority_queue module and we just don't need all of these slow, flaky alternatives.

@SimonSapin
Copy link
Contributor Author

@thestinger is this new sort that you’re planning stable? Also, Timsort’s property of being fast on already-sorted sub-sequences is very interesting for my use case. Is it really slow and flaky?

@thestinger
Copy link
Contributor

Yes, all of the sorts in that module are slow, and none of them would need Clone if they were done well.

I think the merge sort would be stable, as a traditional merge sort and insertion sort are. It should be quite fast on already-sorted sub-sequences, likely faster than Timsort for anything without a very expensive comparison function.

@14427
Copy link
Contributor

14427 commented Oct 3, 2013

The sort is stable. I think it was faster in my benchmarks than mergesort, but they might not have been that representative of actual data.
Also, the original, unsafe implementation for timsort didn't need Clone. It got switched so that it wouldn't be doing any tricks to avoid copies.

@huonw huonw closed this as completed in 48fedcb Dec 22, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants