Added toAscUnfoldable

[joshuahhh]

The current version of toList has the nice undocumented property that it returns key/value pairs in ascending order. Per recent changes to toUnfoldable (#76), I can imagine this implementation being changed in the future, breaking this property. So here's a synonym which assures the user that they will get their key/value pairs in ascending order, together with a test.

In case you were curious: I adapted @sammthomson's benchmark from #76 to a couple different ways to compute sorted & unsorted collections from maps:

This suggests that toList is a faster way to get a sorted collection than sorting the output of toUnfoldable (with list or array output), but it is a slower way to get an unsorted list than running toUnfoldable with list output.

[hdgarrood]

Can you try changing your benchmark so that it uses larger maps (say, up to 10k elements)? I think toList might be quadratic, for a similar reason that toUnfoldable previously was. Because this benchmark only goes up to 60 elements, I don't think we have enough information to conclude that toList is a faster way to get a sorted collection in general.

[joshuahhh]

@hdgarrood Good idea. Here is a plot going out to 10k elements:

It looks like toList scales well.

In case you want a sorted array, rather than a sorted list, the fastest way is still through toList (though the SVG renderer runs out of colors):

(Benchmark code is here.)

But to be clear – my intent with this PR is to establish a clear API with a toAscList function, regardless of how these functions are implemented or what their performance characteristics are.

[joshuahhh]

I noticed that some uses of toList in Map (like eq and compare) assumed that the output of toList was ordered. I switched them to use toAscList.

[hdgarrood]

Ok cool, good to know. I realise what the goal of this PR is, I just wanted to check the assumption it was based on :)

If toUnfoldable is faster than toList (in cases where we don't care about the result being sorted), then there's no reason to have both toList and toAscList, is there?

Also have you tried to come up with an efficient way of implementing this in terms of Unfoldable rather than List? It would be nice to have a generalised toUnfoldableAsc.

Either way though I think we should deprecate toList.

[joshuahhh]

@hdgarrood Your question about toAscUnfoldable was a very good one! Turns out toAscUnfoldable is a small variant on toUnfoldable which actually runs faster than toList:

So here's what I've done:

• Added toAscUnfoldable.
• Removed toAscList and switched all uses of it to toAscUnfoldable using arrays (since they're a bit faster than lists, according to the plots above).
• Switched toList to be an alias of toAscUnfoldable using lists. Feel free to deprecate it, if you prefer!
• Cleaned up some of the tests for toUnfoldable/fromFoldable.

Please take a look. 😃

[paf31]

👍 Looks good to me, and deprecating toList sounds good.

[hdgarrood]

Looks great, I just have a few minor suggestions:

• This will change in the upcoming compiler release with the new Warn class, but for now, the best way we have of deprecating functions is to add a note in the inline doc comments saying something like "DEPRECATED: use toUnfoldable or toAscUnfoldable instead" - could you add that please?
• Could you change all uses of toList in the tests to use either toUnfoldable or toUnfoldableAsc as appropriate?
• I think toList should become a synonym for toUnfoldableAsc since people might be relying on the ordering property.

[joshuahhh]

All those points sound good. I'll make the changes.

Small question: you occasionally say toUnfoldableAsc instead of toAscUnfoldable. I chose toAscUnfoldable to be parallel to toAscList, which is a map function in Haskell. But I don't have a real preference, so let me know if you prefer the other way.

[hdgarrood]

This looks great, thanks very much!

[hdgarrood]

Yeah I'm not particularly fussed about which way around it is, I think it probably does make sense to mirror Haskell if that's the way Haskell does it.