ekmett / kan-extensions

Kan extensions, Kan lifts, the Yoneda lemma, and (co)monads generated by a functor
Other
78 stars 33 forks source link

Added oneShot to Codensity continuation #79

Open Icelandjack opened 6 months ago

Icelandjack commented 6 months ago

Solving issue #78. Rewrote functions to add oneShot to the continuation:

instance Functor (Codensity k) where
  fmap f (Codensity m) = Codensity $ oneShot $ \k -> m (k . f)
Icelandjack commented 6 months ago

Is it worth adding a oneShot to each continuation, like in ghc

  Codensity f <*> Codensity g =
    Codensity $ oneShot (\bfr -> f $ oneShot (\ab -> g $ oneShot (\x -> bfr (ab x))))

Or the one from IOSim which only uses it for the first continuation.

  (<*>) = \df dx -> IOSim $ oneShot $ \k ->unIOSim df (\f -> unIOSim dx (\x -> k (f x)))
RyanGlScott commented 6 months ago

Is it worth adding a oneShot to each continuation, like in ghc

  Codensity f <*> Codensity g =
    Codensity $ oneShot (\bfr -> f $ oneShot (\ab -> g $ oneShot (\x -> bfr (ab x))))

Or the one from IOSim which only uses it for the first continuation.

  (<*>) = \df dx -> IOSim $ oneShot $ \k ->unIOSim df (\f -> unIOSim dx (\x -> k (f x)))

An interesting question. Considering that GHC invented the oneShot trick (as far as I am aware), I'm inclined to use GHC's implementation. That being said, it would be worth opening an issue on the io-sim side to see if there is a deeper reason for this difference (or if this is simply an oversight).

Icelandjack commented 6 months ago

I've fixed it @RyanGlScott, as to your questions I asked Marcin Szamotulski about it and he did the following benchmark: https://github.com/input-output-hk/io-sim/discussions/149. One test drops from 6.7ns to 5.955ns but he says he's going to run a larger simulation to see if it makes a more significant difference.

RyanGlScott commented 6 months ago

Thanks, this is looking better. Two thoughts:

  1. We should definitely cite GHC's Note about the one-shot trick somewhere in the comments, as it's not obvious why you'd want to use oneShot here unless you're familiar with the trick.
  2. Reading that Note, one part of the trick that is not implemented here is using a pattern synonym to ensure that all applications of Codensity make use of oneShot. Shouldn't we do that here as well?
Icelandjack commented 6 months ago

Marcin had mentioned that idea previously

I run an experiment where I added them using a pattern synonym for IOSim, and it was a regression with respect to just adding them manually. It was surprising to me, but I haven't investigated why it was so.

I can write a blurb and link as well, but what is the best format of linking to a note from ghc?

RyanGlScott commented 6 months ago

Wow, the fact that it regressed performance is indeed surprising. I suppose this means that we really should be benchmarking these Codensity changes to see what kind of performance numbers we get—or, if nothing else, we should cite this io-sim discussion (is it online somewhere?) as justification.

I can write a blurb and link as well, but what is the best format of linking to a note from ghc?

I think it would be helpful to write our own Note in the module which defines Codensity, and to include a reference to the Note from the first use of oneShot. Something like:

-- See Note [oneShot] for an explanantion of why we use oneShot below

instance Functor (Codensity (k :: j -> TYPE rep)) where
  fmap f (Codensity m) = Codensity (\k -> m (\x -> k (f x)))
  {-# INLINE fmap #-}

instance Apply (Codensity (f :: k -> TYPE rep)) where
  (<.>) = (<*>)
  {-# INLINE (<.>) #-}

-- ...

{-
Note [oneShot]
~~~~~~~~~~~~~~
The code in this module uses oneShot because ...

The inspiration for this trick comes from ... (cite the GHC Note here)

We deviate slightly from the presentation shown in that Note because
of performance reasons, which are explained in ...
-}