Override memoize for Resource
#3080
Conversation
| override def memoize[A](fa: Resource[F, A]): Resource[F, Resource[F, A]] = { | ||
| Resource | ||
| .makeCaseFull[F, F[(A, Resource.ExitCase => F[Unit])]](poll => | ||
| poll(F.memoize(fa.allocatedCase))) { (memo, exit) => memo.flatMap(_._2.apply(exit)) } | ||
| .map(memo => Resource.eval(memo.map(_._1))) | ||
| } |
There was a problem hiding this comment.
So one issue with this implementation is that it always forces allocation in the release step. Actually this is giving me deja vu.
Still, it seems like with more work even that could be solved.
There was a problem hiding this comment.
So my concern with this is it makes the semantics moderately incoherent, which in turn may have an impact on abstract code. What I'm not convinced of is that this is necessarily wrong, since the memoize semantics we inherit are consistent with the way that fiber lifecycles work, but inconsistent with the way that Resource lifecycles work. Does that make it wrong? Is the memoize implementation upstream assuming something that it shouldn't?
Basically, I feel like we need a bit more of a first-principles argument here and it's eluding me.
| override def memoize[A](fa: Resource[F, A]): Resource[F, Resource[F, A]] = { | ||
| Resource.eval(F.ref(false)).flatMap { allocated => | ||
| val fa2 = F.uncancelable(poll => poll(fa.allocatedCase) <* allocated.set(true)) | ||
| Resource | ||
| .makeCaseFull[F, F[(A, Resource.ExitCase => F[Unit])]](poll => poll(F.memoize(fa2)))( | ||
| (memo, exit) => allocated.get.ifM(memo.flatMap(_._2.apply(exit)), F.unit)) | ||
| .map(memo => Resource.eval(memo.map(_._1))) | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm relatively certain this isn't enough. :-) We should make sure that all of the GenConcurrent tests for memoize are applied to this implementation.
There was a problem hiding this comment.
Hmm, why is that? All the existing tests in MemoizeSpec appear to work, unless I did something dumb.
In what sense? Is there an example of a program you can write with only |
Fixes #3079 (comment). Or so I thought 🤔