lspitzner / pqueue

Haskell priority queue package
Other
15 stars 13 forks source link

Avoid eta expansion by composition #101

Open treeowl opened 1 year ago

treeowl commented 1 year ago

We have a lot of f . unDown getting passed to higher-order functions that can't inline. In situations where GHC doesn't know that f has arity at least 1, the RTS will allocate a closure through which to call f. That's not nice. We should borrow a trick from profunctors:

infixl 8 .#
(.#) :: forall a b c q. Coercible b a => (b -> c) -> q a b -> a -> c
(.#) pbc _ = coerce pbc
{-# INLINE (.#) #-}

-- We may not need this one
infixr 9 #.
(#.) :: forall a b c q. Coercible c b => q b c -> (a -> b) -> a -> c
(#.) _ = coerce (\x -> x :: b) :: forall a b. Coercible b a => a -> b
{-# INLINE (#.) #-}

Then we can write Down #. f to coerce it nicely. The exact definitions in profunctors are carefully arranged to work around some weirdness in Coercible solving back in 7.10 and (IIRC) 8.0.

Then we can write things like

traverseWithKey f (MaxPQ q) = MaxPQ <$> Q.traverseWithKey (f #. unDown) q

We'll also want to implement mapWithKey and mapWithKey' using coercions.

konsumlamm commented 1 year ago

I'm fine with that. Couldn't we also just use coerce f though?

treeowl commented 1 year ago

Maybe. I expect that'll work fine for relatively recent GHC versions, but things were a bit wonky in 7.10 and 8.0, IIRC. Maybe not too wonky for what we're doing here.

treeowl commented 1 year ago

Another good option might be to drop any GHC versions where we can't just use coerce. But doesn't this package have a tradition of trying not to be GHC specific?

konsumlamm commented 1 year ago

But doesn't this package have a tradition of trying not to be GHC specific?

IIRC yes (although I can't find any comment on that rn), but IMO that's a lost cause. GHC is de facto the only Haskell compiler and I'm pretty sure that most packages (including deepseq and indexed-traversable, which this package depends on) don't work with other compilers anyway.