-
Notifications
You must be signed in to change notification settings - Fork 19
Makes Eff Harder, Better, Faster, Stronger #31
base: master
Are you sure you want to change the base?
Conversation
I think this is at least worth considering:
I think having both |
One problem though is that compiler optimized code can blow the stack, while -- Blows the stack?
loop x = do
next <- inc x
loop next
-- Doesn't?
loop x = inc x >>= loop |
I think this is interesting and very cool, but it seems to change a basic library in a way which is incompatible with the original project goals. Namely, it's not minimal, and it's not simple (I don't fully understand it yet, and I've been reading it off and on since you originally posted it in the other repo). I think it needs more documentation for me to understand it properly, for one. It seems like this is purely an optimization, so I think making this fork available in an alternative package set for extensive testing in real world apps would be helpful. That way, we can test other things like how this impacts debugging. Worst case, it's a drop-in replacement for |
Will totally add comments on how things are working . We can hold merging it for some time, try to make this change in some real world apps and see if there are any issues: - "purescript-eff": "^3.0.0",
+ "purescript-eff": "safareli/purescript-eff#fast" In terms of debugging we can even implement custom toString function method for eff nodes we create so we have more options to improve debugging this way. Yes it's drop in replacment of Eff. |
This is output of
|
@natefaubion I guess the do stack safety issue should be handled in compiler. |
@safareli That amounts to a whole program optimization due to mutual recursion. |
package.json
Outdated
@@ -1,13 +1,18 @@ | |||
{ | |||
"private": true, | |||
"scripts": { | |||
"clean": "rimraf output && rimraf .pulp-cache", | |||
"clean": "rimraf output && rimraf .pulp-cache && rimraf bench/output && rimraf bench/.pulp-cache", | |||
"install": "bower i && cd bench && bower i", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd call it installation
so there is no confusion with native used of install
ie if you try use yarn to run this it won't work. yarn install
😞
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or postinstall
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what are you doing here @toastal ? we've provided the least useful feedback 😂
So we can't do much here. But at least it's not a regression. I will add that issue to documentation. |
Update: |
"name": "purescript-eff-aff-bench", | ||
"dependencies": { | ||
"purescript-foldable-traversable": "^3.6.1", | ||
"purescript-minibench": "safareli/purescript-minibench#un-log", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -0,0 +1 @@ | |||
../node_modules/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how useful will be this bench project might be in future, we can also remove it too.
This file is result of running ln -s ../node_modules/ ./node_modules
in bench
folder, so we don't mix bench scripts with root project.
Cont
of safareli/purescript-ef#1fixes #29
Same
do
optimization is still done, here is fragment fromoutput/Test.Main/index.js
:TODO