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

Array creation #64

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

jfmengels
Copy link
Contributor

@jfmengels jfmengels commented Nov 24, 2021

(Builds upon the fix-test-suite branch from #63, only the last commit is important here)

I made a transformer that removes the performance penalty for building Elm Array using something like Array.fromList [ 1, 2, 3 ], by precomputing the resulting the shape of the transformations result. This optimization only applies when encountering list literals. I did this by copying the compiled code that does this transformation inside this repository.

I made 2 benchmarks in JS-land to test the performance:

Screenshot from 2021-11-24 16-31-09

  • https://jsbench.me/zrkwdnkgos (100) items: Result is about 2.5 times faster.
    (Note that the order of the test suite is not the same as above, sorry about the confusion)

Screenshot from 2021-11-24 15-29-31

I do suspect that the benchmark is not representative as it might be over-learning/over-optimizing this short piece of code, but I don't know how to better test this.

I don't know how to get this proposal further than this. I can't run the benchmarks locally (they fail for some reason). I should be able to add this transformer to the configuration that enables/disables transformations, and I'll do that soon, but that's as far as I know how to go without further help.

How do I go about validating that this improvement works out well in practice? And what are the missing steps to get this in?

@jfmengels
Copy link
Contributor Author

Probably worth mentioning that this idea came from @robinheghan's post https://dev.to/skinney/de-throning-the-list-part-boron-185
I only came up with the idea of doing this through elm-optimize-level-2, most credit goes to Robin 😊

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.

1 participant