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

Use List.foldr in Task.sequence #521

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@sgillis

sgillis commented Mar 14, 2016

Using foldl instead of a recursive function prevents that we reach the maximum call stack size when sequencing a lot of tasks.

Use List.foldl in Task.sequence
Using foldl instead of a recursive function prevents that we reach the maximum call stack size when sequencing a lot of tasks.
@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Mar 14, 2016

Contributor

But you are returning the results in the wrong order. That is, the result list will be reversed in comparison to the current implementation.

Contributor

jvoigtlaender commented Mar 14, 2016

But you are returning the results in the wrong order. That is, the result list will be reversed in comparison to the current implementation.

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Mar 14, 2016

Contributor

Use List.foldr instead, and you will also get a non-recursive implementation (using JavaScript looping), but retain correct semantics.

Contributor

jvoigtlaender commented Mar 14, 2016

Use List.foldr instead, and you will also get a non-recursive implementation (using JavaScript looping), but retain correct semantics.

@jvoigtlaender jvoigtlaender changed the title from Use List.foldl in Task.sequence to Use List.foldr in Task.sequence Mar 14, 2016

@turboMaCk

This comment has been minimized.

Show comment
Hide comment
@turboMaCk

turboMaCk Dec 4, 2016

Contributor

This seems like iprovement to me. Original implementation does not use tail call. What is state of this?

Contributor

turboMaCk commented Dec 4, 2016

This seems like iprovement to me. Original implementation does not use tail call. What is state of this?

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Jul 8, 2017

Member

I ended up merging #881 because that's the one I saw first after List.foldr was reimplemented in Elm.

Member

evancz commented Jul 8, 2017

I ended up merging #881 because that's the one I saw first after List.foldr was reimplemented in Elm.

@evancz evancz closed this Jul 8, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment