tail-recursive keys and values

[matthewleon]

This PR exposes the functions toAscUnfoldableKeys and toAscUnfoldableValues, and rewrites the keys and values functions to use them. They are stack-safe, drastically faster, and guarantee ascending order according to the keys' Ord instance.

Benchmarks are included. To compare to the implementations on master, check out this branch: https://github.com/matthewleon/purescript-maps/tree/bench-keys-values

Highlight: keys and values for a Map of 1000000 pairs reduced from 3.80 seconds (!) to 712.57 ms.

[matthewleon]

Note that this has a knock-on effect of speeding up some conversions of Sets, as well, as they depend on keys.

[hdgarrood]

This looks great, but instead of exposing new functions I think I would rather just generalise the existing keys and values during the next round of breaking changes.

Does the behaviour of the current keys and values change with this PR? In particular are they ordered the same way as before?

[hdgarrood]

Also I would like to set a convention of avoiding use of ~> in the core libraries. I don’t think anything in core has a type signature complicated enough that using ~> makes much of a difference, and I have heard reports that it makes reading type signatures quite a bit harder for beginners. There are a few places ~> is used in core already; if people agree with this I’d like to change those.

[matthewleon]

This all sounds very reasonable to me. I'll replace the Unfoldable functions with generalized keys and values. I'll likewise get rid of the ~> from signatures.

Regarding behavior/ordering: I believe the old behavior also had results in order of ascending key. From reading the code, it looks like the old behavior is identical to the new, it just used a BFS rather than a DFS approach and incurred a huge penalty on list concatenation. Please do have a look yourself to confirm this for me.

[matthewleon]

My inclination is to say that Travis is lying and needs to be kicked :)

[matthewleon]

Actually I am wondering here if it makes sense to expose both ascending-order and non-ascending order keys and values if there is a speed difference?

[matthewleon]

I've now cleaned this up with some refactoring and added appropriate tests.

@hdgarrood I think this is ready for review. Because it doesn't change ordering, I believe it's non-breaking. I'd like to get this merged in, if possible, before proceeding to some of the breaking changes/refactors that we've discussed in other issues.

[matthewleon]

Whoops, actually this is breaking due to the generalization of the signatures on keys and values. No rush on this.