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

Replace unsafePerformIO with runST, bonus speed! (Fixes #15) #16

Merged
merged 5 commits into from Feb 18, 2018

Conversation

@axman6
Copy link
Contributor

commented Feb 17, 2018

Replace all uses of unsafePerformIO wth runST. Unexpectedly this has resulted in some serious performance improvements, with some benchmarks now running 4x faster according to criterion. Purity for the win again! I'm a little skeptical about these results so would love someone else to confirm them - specifically the benchmarks set nested/superrecord, get set/superrecord, set rec/superrecord and dummy list/superrecord are now much faster. This might be because GHC no longer has to be careful about having nested IO actions, but that's just speculation.

@agrafix

This comment has been minimized.

Copy link
Owner

commented Feb 17, 2018

Could you also fix the GHCJS build? It seems to need unsafe perform IO somewhere.

Alex Mason Alex Mason
@axman6

This comment has been minimized.

Copy link
Contributor Author

commented Feb 17, 2018

Done, building now.

Alex Mason added some commits Feb 17, 2018

Alex Mason Alex Mason
Alex Mason Alex Mason

@agrafix agrafix merged commit 62560b8 into agrafix:master Feb 18, 2018

3 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: build-ghc82 Your tests passed on CircleCI!
Details
ci/circleci: build-ghcjs Your tests passed on CircleCI!
Details
@agrafix

This comment has been minimized.

Copy link
Owner

commented Feb 18, 2018

Thanks!

@axman6

This comment has been minimized.

Copy link
Contributor Author

commented Feb 19, 2018

@agrafix Did you get a chance to run the benchmarks comparing the old and new implementations? I was very surprised by the performance improvements and wanted to make sure it wasn't just a fluke.

@axman6 axman6 deleted the axman6:no-unsafePerformIO branch Feb 19, 2018

@agrafix

This comment has been minimized.

Copy link
Owner

commented Feb 20, 2018

Not yet, but I would certainly assume that they perform similarly. Got a lot on my plate atm, but hopefully will get the change to dig into this again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.