Use Endo [a] for universe/On/Of implementation#994
Use Endo [a] for universe/On/Of implementation#994RyanGlScott merged 2 commits intoekmett:masterfrom
Conversation
| universeOf :: Getting [a] a a -> a -> [a] | ||
| universeOf l = go where | ||
| go a = a : foldMapOf l go a | ||
| universeOf :: Getting (Endo [a]) a a -> a -> [a] |
There was a problem hiding this comment.
Presumably the changes to the type signatures in this module warrant a major version bump?
There was a problem hiding this comment.
OK. Can you mention these changes in the CHANGELOG, along with a description of how to migrate code that might be broken by this change? (I'd do so myself, but I'm not sure I understand which situations would break due to these changes.)
There was a problem hiding this comment.
If people have Getting [a] values they pass around, they will need to use Getting (Endo [a]), but I doubt anyone does that. Or if they define own combinators (which take in Getting [a] a a), they would need to change signature. So in short: fix type errors GHC tells you to fix.
(It's a unfortunate that Getting r s a has that r argument, leaky implementation detail)
There was a problem hiding this comment.
Here is a proposed addition to the CHANGELOG:
5.2 [????.??.??]
----------------
* The type of `universeOf` has changed:
```diff
-universeOf :: Getting [a] a a -> a -> [a]
+universeOf :: Getting (Endo [a]) a a -> a -> [a]
```
In many cases, using `Endo [a]` over `[a]` improves performance. Most call
sites to `universeOf` will not be affected by this change, although you may
need to update your code if you define your own combinators in terms of
`universeOf`.
Does this sound right? If so, I can add this after merging the PR.
There was a problem hiding this comment.
that's great! thanks.
30f3de2 to
c2ecc35
Compare
I added a benchmark which is maybe exaggerated but highlights the problem:
I don't know why
Databased plates are slower though, that is weird.