Adds timsort and fix issue #3265

[14427]

No description provided.

[brson]

Besides being unsure about the unsafe code this looks good to me.

It would be nice to refactor std::sort a bit more in the future. le methods can be replaced by Ord. Each of the sorts should probably get their own module. There should probably be some kind of sorting trait. But that all can be done later. Also, we probably don't need this many sort functions.

[brson]

Did you compare the performance of this sort to any of the existing ones?

[14427]

It looks like while unique pointers work fine, the sort seems to break the refcount of managed pointers. MergeState's destructor calls set_len, so even if the task fails, tmp get cleaned up without double-freeing or running the destructors of anything left in it. The performance is better than merge_sort in all the cases I tested, and everywhere that quick_sort is faster, quick_sort3 beats it.

[14427]

Here are the perf numbers:
https://gist.github.com/3808689

The benchmark is a translation of python's sortperf.py sort benchmark

[14427]

Here's python's source: http://code.google.com/p/unladen-swallow/source/browse/trunk/Lib/test/sortperf.py
The quicksort numbers don't go on as long, because they run out of stack

[14427]

I also checked that dtors run, so other then leaking managed pointers some of the time, everything works.

[brson]

Those are very promising numbers!

[14427]

I also double checked the managed pointer thing, and it it turns out that it works, I just wrote my refcount test wrong. It always expected a count of 1, when several tests duplicated elements, increasing the refcounts.

[brson]

This is going to need more tests. It's a complicated algorithm and all the included tests are on scalar data with small vectors. It looks to me like all the test cases cause timsort to immediately bail to the fast path, so there is no coverage for the difficult parts.

Needs tests with both managed and owned boxes, and with cases that cover the different strategies used by timsort. I know it's a bit more work, but it's important.

FWIW, I've poked at writing more tests a few times, but don't have enough time right now to keep at it.

[14427]

I'm hitting some ICEs with the test suite. Just updated my branch to see if it fixes things.

[14427]

Here's the backtrace from the ICE: https://gist.github.com/3834914
I'll see if I can narrow down what causes it and file an issue.
I'll also submit the rest of the testsuite commented out.

[graydon]

Tricky. On the one hand, I'm excited to see such improved performance; on the other it seems like "unsafe required to do sorting" is a bad sign, and I'm pretty hesitant to merge something ostensibly high-level like a sorting algorithm that has unsafe code all through it.

I'm curious exactly how much the performance difference just has to do with hitting native memcpy. If it's "most", we should probably take a step back and figure out how to make vec-to-vec copies in general speed-competitive.

[14427]

The unsafe code isn't for performance, it's so the code doesn't do any copies anywhere, so that it works on non-copyable data structures. Most of the performance difference comes from doing less compares (about 40 times less than quick_sort3 on random data). The performance of tim_sort is actually about the same as quick_sort3 on random data when I added a tls get and set call to the compare when getting the compare numbers.

[14427]

Broken pending #3821

[brson]

This builds and tests for me now. On IRC we discussed reworking this to not use unsafe code, at the expense of adding a Copy bound, and I think that would make everyone more comfortable.

[nikomatsakis]

+1 to removing unsafe code for the time being, at least.

[14427]

The current code is generating a lot of warnings about instantiating non-implicitly copyable types in the tests that I'm not sure how to fix. Other than that the Copy code is done.

[brson]

Merged. Thanks!

[graydon]

This patch doesn't have enough contact information to determine the author (and the github account associated is pretty sparse). Can we get a name and email address?