-
Notifications
You must be signed in to change notification settings - Fork 19
[breaking] traverseE #22
Conversation
|
Using this in https://github.com/matthewleon/purescript-maps/tree/StrMap-fromFoldable-traverseE Getting about 25% speed gain over constructing an intermediate array on traversal of a list of 100000 elements. |
| "dependencies": { | ||
| "purescript-prelude": "^3.0.0" | ||
| "purescript-prelude": "^3.0.0", | ||
| "purescript-foldable-traversable": "^3.3.0" |
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'm not sure we should add this dependency here.
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.
Let me have a think about how we can avoid it.
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.
It also feels a bit odd to me. But then so does putting it in purescript-foldable-traversable. Curious to read your idea.
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.
One option is to put a traverseRec/traverseRec_ into Foldable/Traversable, which would use MonadRec, and then to implement it for Array, List, etc., but that seems unpleasant for a bunch of reasons.
I'm going to think about how we might be able to create something general for consuming things one at a time without creating garbage.
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.
Yes... Without running any benchmarks, my guess is that the tailRecEff machinery here would be significantly less efficient than this PR: https://github.com/purescript/purescript-tailrec/blob/master/src/Control/Monad/Rec/Class.purs
Generally speaking, having some kind of traverseRec seems nice. But I feel like for Eff in particular, it makes sense to have something optimized.
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.
Another thought on traverseRec: separate typeclasses for it. That makes the change un/less breaking.
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.
Yes, I think that's the simplest approach. It's pretty imperative, but then so is Eff generally. Something specific to Eff like
class Functor f <= Iterable f where
forEach_ :: forall eff a b. (a -> Eff eff b) -> f a -> Eff eff Unit
forEach :: forall eff a b. (a -> Eff eff b) -> f a -> Eff eff (f b)should provide a nice sweet spot between "too general, instances can't be made efficient" and "too specific, not enough instances".
Following discussion at purescript-deprecated/purescript-eff#22 (comment)
Should make it easier and more efficient to write stack-safe traversals. The need for this arose here: purescript-deprecated/purescript-maps#110
It seemed inefficient to require the use of a temporary, intermediate array to avoid building up
Effthunks.