-
Notifications
You must be signed in to change notification settings - Fork 506
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add specialized signature of memoize
to Resource
#3513
Comments
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)))
}
}
}
} |
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")
}
|
Perhaps, I'm missing something.. But why should |
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. |
Hmm.. I see you point and that makes sense, but just to clarify.. Let's try to describe
but not
is that correct? |
Hm, I'm not sure if this makes sense in general. The observation here is that, for the special case of |
On the second thought,
I would guess (simply by looking at the signature), it should occur exactly after we leave the inner resource's scope, 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
At first glance (and if I'm not missing something), it could make some sense... |
@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 |
@SystemFw, actually I meant is that the entire Resource with its But I understand your point now: we can have more that 1 different Ok, I got it now, thank you for the clarification, it was helpful! |
So currently
memoize
onResource
returns aResource[IO, Resource[IO, A]]
. The thing is, that inner resource is not really necessary (the implementation is just aResource.eval(...)
) and it makes the ergonomics a lot worse. The ideal signature would look more likeResource[IO, IO[A]]
.Besides
memoize
, I wonder if we can rescue some of the other method signatures as well, that return extremely hairy nestedResource
s. It's pretty cool thatResource
implementsAsync
with all these bells-and-whistles, but the method signatures aren't always very ergonomic when working directly with concreteResource
.The text was updated successfully, but these errors were encountered: