PLT-1539: Use foldr for Foldable
#5179
Conversation
|
Seems good to me. I doubt anyone is defining their own |
| any p = getSum #. foldMap (Sum #. p) | ||
| any p = foldr f False | ||
| where | ||
| f a acc = p a || acc |
There was a problem hiding this comment.
Try inlining f manually. base has a reason for pulling it out (and marking with INLINE, which you also might try), but we at the moment probably don't and there's a chance it would be sufficient for GHC not to create a thunk for acc.
There was a problem hiding this comment.
I've tried inlining f and || manually, as well as oneShot, inline and {-# INLINE #-}, but without telling the GHC to run the inliner (sm_inline = False), nothing I've tried managed to make GHC turn
(\a acc -> if p a then True else acc) x (go xs)into
if p x then True else go xsWe do run GHC's pre-inliner, but apparently that doesn't do it.
There was a problem hiding this comment.
We need to propagate the INLINE, then it might work.
| let !acc : Bool = go xs in Bool_match | ||
| (ifThenElse | ||
| {Bool} | ||
| (lessThanEqualsInteger 1 x) | ||
| False True) {all dead. Bool} | ||
| (/\dead -> acc) (/\dead -> | ||
| False) | ||
| {all dead. dead}) | ||
| {all dead. dead} |
There was a problem hiding this comment.
So everything works just fine except GHC makes a thunk for go xs and pulls it out of ifThenElse. And we interpret it as a strict binding.
| where | ||
| go = \case | ||
| [] -> z | ||
| x : xs -> f x (go xs) |
There was a problem hiding this comment.
What about oneShot (f x) (go xs) then? I know we probably shouldn't try to do fancy things to get efficient compilation, because it's way too unreliable, but maybe a few general policies here and there would be fine?
| 3 (c | ||
| in | ||
| letrec !go : List integer -> Bool | ||
| = \(ds : List integer) -> |
There was a problem hiding this comment.
prettyprinting improvement: always put the binding on the next line so we can indent it just 2 instead of 7 for letrec
| {integer} ds {all dead. Bool} (/\dead -> True) | ||
| (\(x : integer) (xs : List integer) -> | ||
| /\dead -> | ||
| let !acc : Bool = go xs in Bool_match |
There was a problem hiding this comment.
why didn't we break at the in here?
| in | ||
| let !eta : List integer | ||
| = (let a = List integer in \(c : integer -> a -> a) (n : a) -> | ||
| c |
There was a problem hiding this comment.
this is hilarious but I'm not sure what we could do better
|
I'll capture these prettyprinter improvements in a ticket. I also noticed That seems messed up. |
|
I just went ahead and did it: #5184 Not sure what's up with the |
Suggested by @effectfully in #5168 (comment)
This doesn't make
anyand friends short-circuit, but does reduce the cost (sometimes dramatically) across the board, even more so than this suggestion for improving foldMap.