Fix List.take causing stack overflow

[nphollon]

Fixes #601

List.take causes a runtime error if we take approx. 4000-5000 items.

import Html
main = [ 1 .. 6000 ] |> List.take 4000 |> toString |> Html.text

I think this is because the recursive definition of List.take does not get optimized by the compiler. I have rewritten it to use foldr instead of recursion.

[process-bot]

Thanks for the pull request! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

[process-bot]

Thanks for the pull request! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

[evancz]

I believe this version costs O(n) in all cases. So if I say List.take 2 [1..1000] it will traverse 1000 elements. I believe there is a way to write this tail recursively that is O(k) where k is the amount you want to take. That said, this will necessarily reverse the list, meaning you will have to do a reverse on it, increasing allocation.

It is not clear to me that this is a win. This problem exists in OCaml and SML as well. There is no real trick to fix it besides letting the stack be bigger or using a more appropriate data structure for your data set.

[evancz]

I believe this version costs O(n) in all cases. So if I say List.take 2 [1..1000] it will traverse 1000 elements. I believe there is a way to write this tail recursively that is O(k) where k is the amount you want to take. That said, this will necessarily reverse the list, meaning you will have to do a reverse on it, increasing allocation.

It is not clear to me that this is a win. This problem exists in OCaml and SML as well. There is no real trick to fix it besides letting the stack be bigger or using a more appropriate data structure for your data set.

[nphollon]

So I am building a collision detection library that uses a bunch of recursive algorithms. When I started encountering stack overflows, I wasn't surprised; I figured I'd probably done something dumb. I spent a couple days tracking down the problem (as chronicled on elm-discuss). Finding out that the issue was in the standard library was pretty annoying.

I think its a big win to get rid of the stack overflow. When somebody is debugging a runtime error, elm-lang/core is going to be very far down on the list of suspects. Even if this isn't encountered that often, its going to be a time sink.

Any of these I think would be better than the status quo:

• replacing List.take with a tail-recursive version that is O(k) but reverses the order of the items.
• coupling the above function with a list reversal.
• removing List.take
• adding a note to the documentation that List.take only works for small values of n.

[nphollon]

So I am building a collision detection library that uses a bunch of recursive algorithms. When I started encountering stack overflows, I wasn't surprised; I figured I'd probably done something dumb. I spent a couple days tracking down the problem (as chronicled on elm-discuss). Finding out that the issue was in the standard library was pretty annoying.

I think its a big win to get rid of the stack overflow. When somebody is debugging a runtime error, elm-lang/core is going to be very far down on the list of suspects. Even if this isn't encountered that often, its going to be a time sink.

Any of these I think would be better than the status quo:

• replacing List.take with a tail-recursive version that is O(k) but reverses the order of the items.
• coupling the above function with a list reversal.
• removing List.take
• adding a note to the documentation that List.take only works for small values of n.

[evancz]

I think Yaron outlines a very attractive way to approach this kind of problem here. I read it ages ago, but only really remembered the "recursion unrolling" trick. I was going to suggest using this trick to make the stack deeper by 5x, but the post outlines a way better solution than that! If we can show the numbers work nicely in Elm, I think it makes sense to go in this direction across the board.

What do you think of that approach? Would you be willing to do some profiling to see if you can make the performance / memory tradeoffs work out favorably?

[evancz]

I think Yaron outlines a very attractive way to approach this kind of problem here. I read it ages ago, but only really remembered the "recursion unrolling" trick. I was going to suggest using this trick to make the stack deeper by 5x, but the post outlines a way better solution than that! If we can show the numbers work nicely in Elm, I think it makes sense to go in this direction across the board.

What do you think of that approach? Would you be willing to do some profiling to see if you can make the performance / memory tradeoffs work out favorably?

[nphollon]

I think that approach sounds really nice. Gives a fast result for the most common use cases, and switches to an "at least it works" implementation for large lists. I will do some profiling and get back to you.

[nphollon]

I think that approach sounds really nice. Gives a fast result for the most common use cases, and switches to an "at least it works" implementation for large lists. I will do some profiling and get back to you.

[evancz]

Cool, I am going to close this for now. New issue or PR when you find out more! Fine to continue discussion here if necessary though.

[evancz]

Cool, I am going to close this for now. New issue or PR when you find out more! Fine to continue discussion here if necessary though.