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

[WIP] Optimize List runtime representation with a stack #2122

Closed
wants to merge 4 commits into from

Conversation

alfonsogarciacaro
Copy link
Member

Note this PR is for master not nagareyama (codename for Fable 3) so if this gets merged it will be published for Fable 2.

While random-thinking (is that an actual expression?) this weekend, it came to my mind that in Fable we're using a specific type for lists instead of a simple union, but this type is not particularly optimized. I tried to find how we could improve that by taking the following into consideration:

  • Lists are widely used in F# as it's the default collection (at least the one with terser syntax).
  • Most of the time users just want an immutable collection and they don't need an actual linked list.
  • The FSharp.Core List module is usually used to work on lists (vs using a recursive function to fold the list)
  • Most users are aware (hopefully) that adding elements to the beginning of the list is faster than adding them to the end.

So I decided to implement an internal stack to store the items, while the List object itself become a simple pointer to a position in that stack. In most cases a single List pointer should be enough, but additional ones will be created on-the-fly when traversing the list manually. When adding items to the beginning of the list, if the affected list points to the top of the stack we just reuse it, otherwise we create a new stack. This could require extra memory in some cases (e.g. if we have a big list, then go to the last element to the list and grow another list from there) but hopefully it shouldn't happen very often.

I'm using a reverted JS array for the stack (though it makes it a bit harder for debugging) because Array.push should be much faster than inserting at the beginning.

Later I will add some unsafe helpers (that access directly the internal stack) to the List module to optimize the most used functions (fold, map, append) and try to make some benchmarks.

@ncave As you're the performance expert 😉 could you please have a look and let me know if this makes any sense to you?

@gitpod-io
Copy link

gitpod-io bot commented Jul 19, 2020

@ncave
Copy link
Collaborator

ncave commented Jul 19, 2020

@alfonsogarciacaro Chalk it to complete coincidence if you must, but I've just spent the last few days thinking (and testing) the same exact thing (literally the same, reverse dynamic arrays as lists). Minds thinking alike and all that, but the exact timing is intriguing, to say the least. I'll take a look.

Also, I've been looking into the possibility to replace the Seq module with the F# implementation (pretty much as it is in FSharp.Core), so we don't have to use iterators when downstream compilers don't support JS iterators (e.g. assemblyscript).
Not a priority right now, just an option to keep in mind (ideally all of fable-library can be in F# one day).

@ncave
Copy link
Collaborator

ncave commented Jul 19, 2020

@alfonsogarciacaro

I only ran one big benchmark (Fable compiled with Fable-js), but unfortunately it looks like this PR results in 20% slower performance. Not sure why exactly, it certainly sounds like a good idea that should be performing better.

@ncave
Copy link
Collaborator

ncave commented Jul 19, 2020

@alfonsogarciacaro Not caching the _tail improves it a little bit, but not much (from 20% slower to just 17% slower). See #2123.

@krauthaufen
Copy link
Collaborator

Hey, the stack idea sounds promising, but doesn't it require copying the array, once it's full? Another concern would be the waste of memory, due to rounding up the array's length but that may be acceptable.
While the linked-list has obvious caveats i think it will not easily be "beatable" in terms of Cons performance...

@ncave
Copy link
Collaborator

ncave commented Jul 20, 2020

@alfonsogarciacaro @krauthaufen Just to clarify, the lower perf listed above is without any further optimizations in the List module that are made possible by this PR. Still working on that, will post new perf measurements after it's done.

@alfonsogarciacaro
Copy link
Member Author

Superseded by #2124, moving the discussion 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

Successfully merging this pull request may close these issues.

3 participants