typelevel / cats-effect

The pure asynchronous runtime for Scala
https://typelevel.org/cats-effect/
Apache License 2.0
2k stars 513 forks source link

Add specialized signature of `memoize` to `Resource` #3513

Open armanbilge opened 1 year ago

armanbilge commented 1 year ago

So currently memoize on Resource returns a Resource[IO, Resource[IO, A]]. The thing is, that inner resource is not really necessary (the implementation is just a Resource.eval(...)) and it makes the ergonomics a lot worse. The ideal signature would look more like Resource[IO, IO[A]].

Besides memoize, I wonder if we can rescue some of the other method signatures as well, that return extremely hairy nested Resources. It's pretty cool that Resource implements Async with all these bells-and-whistles, but the method signatures aren't always very ergonomic when working directly with concrete Resource.

arturaz commented 1 year ago

As a workaround, you can define this (credit to @armanbilge !)

import cats.syntax.all.*
import cats.effect.{Concurrent, Resource}

extension[F[_], A] (resource: Resource[F, A]) {
  /**
   * As [[cats.effect.kernel.syntax.GenConcurrentOps_.memoize]] but with a type signature that is more ergonomic.
   *
   * [[https://discord.com/channels/632277896739946517/632278585700384799/1089601070428323950]]
   * */
  def memoize2(using F: Concurrent[F]): Resource[F, F[A]] = {
    Resource.eval(F.ref(List.empty[Resource.ExitCase => F[Unit]])).flatMap { release =>
      val fa2 = F.uncancelable { poll =>
        poll(resource.allocatedCase).flatMap { case (a, r) => release.update(r :: _).as(a) }
      }
      Resource
        .makeCaseFull[F, F[A]](poll => poll(F.memoize(fa2))) { (_, exit) =>
          release.get.flatMap(_.foldMapM(_(exit)))
        }
    }
  }
}
armanbilge commented 1 year ago

Well, I wouldn't really recommend inlining that rather complex implementation and opting out of future bug-fixes :)

Instead I would recommend:

def memoize2[F[_], A](r: Resource[F, A])(implicit F: Concurrent[F]): Resource[F, F[A]] =
  Concurrent[Resource[F, *]].memoize(r).map {
    case Resource.Eval[F @unchecked, A @unchecked](fa) => fa
    case _ => throw new AssertionError("should never happen")
  }
satorg commented 1 year ago

Perhaps, I'm missing something.. But why should memoize for Resource result to Resource[F, F[A]] but not F[Resource[F, A]]? I mean, when we call memoize for some F[A] and get F[F[A]] as a result, it is the inner F[A] that gets memoized, isn't it? Whereas the outer F[...] is required just to keep memoize referentially transparent...

armanbilge commented 1 year ago

but not F[Resource[F, A]]

When should the resource be released in this case? :)

The idea of memoizing is that running the inner effect a second time should re-use the result of the first execution. So for a resource, the lifecycle of the memoized value must belong to some outer scope.

satorg commented 1 year ago

Hmm.. I see you point and that makes sense, but just to clarify..

Let's try to describe memoize in terms of abstract effect types, F[_] and G[_]. So you mean that the memoize function assumes in general

F[A] => F[G[A]]`

but not

F[A] => G[F[A]]

is that correct?

armanbilge commented 1 year ago

Let's try to describe memoize in terms of abstract effect types, F[_] and G[_].

Hm, I'm not sure if this makes sense in general. memoize is defined on Concurrent as having the signature F[A] => F[F[A]]. Generally speaking, that's all that is assumed.

The observation here is that, for the special case of Resource, we can implement the desired semantics with the signature Resource[F, A] => Resource[F, F[A]].

satorg commented 1 year ago

On the second thought,

but not F[Resource[F, A]]

When should the resource be released in this case?

I would guess (simply by looking at the signature), it should occur exactly after we leave the inner resource's scope, i.e if we assume

def memoize(ra: Resource[F, A]): F[Resource[F, A]]

then it should be safe to assume that

val res: Resouce[F, A] = Resorce.make(acquireA)(releaseA)

memoize(res).flatMap { (memRes: Resource[F, A]) =>
  memRes
    .use { a => // we can get here only once
      handleA(a) // assume `handleA: A => F[B]`
    }
    .flatMap { (b: B) =>
      // before we get here, `a` gets released with `releaseA`
      // moreover `releaseA` is called only once
      // therefore, we can get here only once and only after `a` has released
      handleB(b)
    }   
}

In other words, the entire Resource as an effect should be memoized (along with its release):

At first glance (and if I'm not missing something), it could make some sense...

SystemFw commented 1 year ago

@satorg that doesn't make sense to me, you've just described how a non-memoized Resource would behave. Think about this: what happens if I call memRes.use twice inside that memoize(res).flatMap { block?

satorg commented 1 year ago

@SystemFw, actually I meant is that the entire Resource with its use block should behave as a single memoized effect. I would say, it is not the same as a non-memoized Resource.

But I understand your point now: we can have more that 1 different use blocks for the same memoized Resource, and that creates ambiguity – which of the use paths should be taken then. Apparently, it is not the issue with a regular memoized F.

Ok, I got it now, thank you for the clarification, it was helpful!