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

Rewrite Flatten to be stack safe #246

Closed
RawToast opened this issue Mar 12, 2020 · 2 comments
Closed

Rewrite Flatten to be stack safe #246

RawToast opened this issue Mar 12, 2020 · 2 comments

Comments

@RawToast
Copy link
Contributor

RawToast commented Mar 12, 2020

The current Flatten implementation for lists (AFAIK) is not stack-safe. I've experienced stack issues flattening very large datasets -- where I had to write my own flatten method instead. I believe this is because it uses List.concat or @ under the hood, which is not safe, but struggle to read the Bs-Abstract/Belt code.

Methods like this should be rewritten to be safe. In the Scala standard library, this is often done using mutation-based implementations but if we can use tail recursive methods then that's good too!

On the back of this, it might be worth creating new methods for many list functions (e.g. concat) that are stack safe.

This might be fixed by #166 if that's the case, you can close this issue 😸

@andywhite37
Copy link
Member

andywhite37 commented Mar 12, 2020

This is a legit issue - there are a number of things that aren't stack safe in the library. MonadRec could potentially solve some of the higher-level things, but we'd be happy to accept one-off PRs that fix individual functions like flatten with more optimized or tail-recursive implementations.

We might be able to address some of these over time, but I've been busy with other things the past few months, so I haven't had a ton of time to dedicate to this.

@andywhite37
Copy link
Member

I'll close this in favor of #247, since there is a more comprehensive list there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants