Skip to content
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

Open
armanbilge opened this issue Mar 26, 2023 · 9 comments · May be fixed by #3890
Open

Add specialized signature of memoize to Resource #3513

armanbilge opened this issue Mar 26, 2023 · 9 comments · May be fixed by #3890

Comments

@armanbilge
Copy link
Member

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
Copy link
Contributor

arturaz commented Mar 26, 2023

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
Copy link
Member Author

armanbilge commented Mar 26, 2023

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
Copy link
Contributor

satorg commented Mar 27, 2023

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
Copy link
Member Author

armanbilge commented Mar 27, 2023

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
Copy link
Contributor

satorg commented Mar 27, 2023

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
Copy link
Member Author

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
Copy link
Contributor

satorg commented Mar 27, 2023

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):

  • its acquire is called once
  • its acquired value is used once
  • after the value has used, it is released once as well
  • to the next operation after use (which does A => F[B]) the memoized resource is observed as a regular memoized F[B]

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

@SystemFw
Copy link
Contributor

SystemFw commented Mar 27, 2023

@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
Copy link
Contributor

satorg commented Mar 27, 2023

@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!

Daenyth added a commit to Daenyth/cats-effect that referenced this issue Nov 8, 2023
@Daenyth Daenyth linked a pull request Nov 8, 2023 that will close this issue
@armanbilge armanbilge linked a pull request Nov 8, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants