-
Notifications
You must be signed in to change notification settings - Fork 26
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
Added more efficient unionWith
, eliminate transformation Map
to List
in Foldable{WithIndex}
instances.
#60
Added more efficient unionWith
, eliminate transformation Map
to List
in Foldable{WithIndex}
instances.
#60
Conversation
…List` in `Foldable{WithIndex}` instances
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.
Could you check what the benchmarks are if the Foldable
instances are updated?
With the
|
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.
Thanks @xgrommx.
Would you mind pushing a changelog entry stating that the speed was improved?
@@ -130,14 +130,32 @@ instance bindMap :: Ord k => Bind (Map k) where | |||
bind m f = mapMaybeWithKey (\k -> lookup k <<< f) m | |||
|
|||
instance foldableMap :: Foldable (Map k) where | |||
foldl f z m = foldl f z (values m) | |||
foldr f z m = foldr f z (values m) | |||
foldMap f m = foldMap f (values m) |
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.
Should we implement values
in terms of foldr
? Seems like values
being accidentally quadratic is a huge problem.
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 do u mean?
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.
He means the values
function. Since it produces a List
, and each list part is concatenated via <>
, which traverses over the list on the left side again (e.g. 1 : 2 : 3 : Nil <> 4 : Nil
), then there's room for optimization 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.
In other words, rather than building multiple lists that get appended together, which requires iterating through list parts a second time, we could just build the final list in one go, no traversals needed:
values :: forall k v. Map k v -> List v
values Leaf = Nil
-values (Two left _ v right) = values left <> pure v <> values right
+values (Two left _ v right) = foldr Cons (Cons v (foldr Cons Nil right)) left
-values (Three left _ v1 mid _ v2 right) =
- values left <> pure v1 <> values mid <> pure v2 <> values right
+values (Three left _ v1 mid _ v2 right) =
+ foldr Cons (Cons v1 (foldr Cons (Cons v2 (foldr Cons Nil right) mid))) left
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.
Ha! I'm dumb. Actually, values
is just foldr Cons Nil
.
values :: forall k v. Map k v -> List v
values = foldr Cons Nil
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.
How do you like them apples?
Map
===
values
---------------
Map2a0bff.values: big map (1000000)
mean = 2.89 s
stddev = 302.43 ms
min = 2.52 s
max = 3.43 s
M.values: big map (1000000)
mean = 278.44 ms
stddev = 4.57 ms
min = 271.75 ms
max = 287.72 ms
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.
See #61
This is really great work! Thanks! |
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.
Fantastic work! Thank you!
Here benchmark results