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

env: remove refinement types from default environment #464

Closed
wants to merge 1 commit into from

Conversation

davidchambers
Copy link
Member

Commit message:

This dramatically lowers the performance cost of type checking collections in some cases.

The difference is staggering in some cases.

~/github.com/sanctuary-js/sanctuary (master)
$ time node -e 'const S = require("./"), xs = []; for (let id = 1; id <= 8; id += 1) xs.push({id}); S.I(xs)'

real	1m9.221s
user	1m8.379s
sys	0m0.790s
~/github.com/sanctuary-js/sanctuary (davidchambers/env)
$ time node -e 'const S = require("./"), xs = []; for (let id = 1; id <= 1000; id += 1) xs.push({id}); S.I(xs)'

real	0m0.450s
user	0m0.456s
sys	0m0.021s

Increasing the length of xs from 8 to 9 on master results in the heap consuming all available memory on my computer. On davidchambers/env increasing the length of xs from 8 to 1000 only adds about 200 milliseconds to the run time.

Removing refinement types from the default environment will result in less informative error messages in some cases (but much clearer error messages in certain other cases). @Avaq has proposed that sanctuary-def should explicitly support refinement types (sanctuary-js/sanctuary-def#162). Were it to do so, refinement types could be ignored when resolving type variables, but could be considered when enumerating a value's types in an error message. The best of both worlds!

Many thanks to @mfidemraizer for providing a minimal test case which made it easy to diagnose this long-standing issue. 👏

This dramatically lowers the performance cost of type checking
collections in some cases.
@mfidemraizer
Copy link

@davidchambers No problem :D

@davidchambers
Copy link
Member Author

Closing in favour of sanctuary-js/sanctuary-def#177

@davidchambers davidchambers deleted the davidchambers/env branch December 10, 2017 17:18
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