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

sort and sortBy are not stack safe #192

Open
milesfrain opened this issue Jan 13, 2021 · 4 comments
Open

sort and sortBy are not stack safe #192

milesfrain opened this issue Jan 13, 2021 · 4 comments
Labels
type: enhancement A new feature or addition.

Comments

@milesfrain
Copy link
Contributor

sort $ range 1 1000000
/home/miles/projects/purescript/lists/output/Data.List/index.js:287
                            return as(new Data_List_Types.Cons(a, ys));
                                      ^
RangeError: Maximum call stack size exceeded
    at $tco_var_as (/home/miles/projects/purescript/lists/output/Data.List/index.js:287:39)

Should we swap out this current version (which I assume depends on Haskell's laziness) for something that just reuses Array's sortBy? I think that might end up being faster even in situations where we're not having stack issues.

-- | Sort the elements of a list in increasing order, where elements are
-- | compared using the specified ordering.
sortBy :: forall a. (a -> a -> Ordering) -> List a -> List a
sortBy cmp = mergeAll <<< sequences
-- implementation lifted from http://hackage.haskell.org/package/base-4.8.0.0/docs/src/Data-OldList.html#sort
where
sequences :: List a -> List (List a)
sequences (a : b : xs)
| a `cmp` b == GT = descending b (singleton a) xs
| otherwise = ascending b (a : _) xs
sequences xs = singleton xs
descending :: a -> List a -> List a -> List (List a)
descending a as (b : bs)
| a `cmp` b == GT = descending b (a : as) bs
descending a as bs = (a : as) : sequences bs
ascending :: a -> (List a -> List a) -> List a -> List (List a)
ascending a as (b : bs)
| a `cmp` b /= GT = ascending b (\ys -> as (a : ys)) bs
ascending a as bs = ((as $ singleton a) : sequences bs)
mergeAll :: List (List a) -> List a
mergeAll (x : Nil) = x
mergeAll xs = mergeAll (mergePairs xs)
mergePairs :: List (List a) -> List (List a)
mergePairs (a : b : xs) = merge a b : mergePairs xs
mergePairs xs = xs
merge :: List a -> List a -> List a
merge as@(a : as') bs@(b : bs')
| a `cmp` b == GT = b : merge as bs'
| otherwise = a : merge as' bs
merge Nil bs = bs
merge as Nil = as

@milesfrain
Copy link
Contributor Author

milesfrain commented Jan 13, 2021

I'm thinking something like this would work:

sortBy :: forall a. (a -> a -> Ordering) -> List a -> List a
sortBy cmp = fromFoldable <<< Array.sortBy cmp <<< Array.fromFoldable

But it's very slow. It should be much faster if we change Array.fromFoldable to use foldl instead. Here's the existing version:

fromFoldable :: forall f. Foldable f => f ~> Array
fromFoldable = fromFoldableImpl foldr

https://github.com/purescript/purescript-arrays/blob/463dcacb99455de5e28ac4a3312bb795f293d2d9/src/Data/Array.purs#L154-L155


Edit: Another option, but still slower than the original list version (up to 10k elements, then list version overflows).

sortBy :: forall a. (a -> a -> Ordering) -> List a -> List a
sortBy cmp = fromFoldable <<< Array.sortBy cmp <<< toUnfoldable

@hdgarrood
Copy link
Contributor

Have you benchmarked what effect it would have by swapping fromFoldable to use foldl rather than foldr?

@hdgarrood
Copy link
Contributor

FYI the Haskell version depends more on the GHC runtime being smarter about stack usage, which is separate from laziness. In Haskell it’s very rare that you have to worry about stack overflows, because of how the GHC runtime works.

@elldritch
Copy link

I just ran into this while solving Advent of Code. Any recommended workarounds in the meantime?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A new feature or addition.
Projects
None yet
Development

No branches or pull requests

4 participants