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 FiberLocal.Make #1821

Open
kubukoz opened this issue Mar 26, 2021 · 11 comments
Open

Add FiberLocal.Make #1821

kubukoz opened this issue Mar 26, 2021 · 11 comments

Comments

@kubukoz
Copy link
Member

kubukoz commented Mar 26, 2021

Follow-up to #1393.

I think users will likely want to create fiber locals for an abstract effect. We can surely create them in IO and other effects that have this functionality, it is probably possible for monad transformers too (I might be missing something).

It would be great to have an abstraction for this that people can depend on instead of sticking to IO:

object FiberLocal {
  trait Make[F[_]] {
    def make(a: A): F[FiberLocal[F, A]]
  }

  object Make {
    implicit val makeForIO: Make[IO] = ...
    
    implicit def makeForOptionT[F[_]: Make]: Make[OptionT[F, *]] = ...
  }
}
@djspiewak
Copy link
Member

I'm definitely not a fan of this. The problem is that this is a false abstraction: fiber locals in other IOs behave entirely differently from CE3's, and from each other. Users can't meaningfully write code which is generic over all of them. I would rather punt the problem downstream a bit and allow users to create a la carte abstractions (similar to UnsafeRun in weaver and in cats-effect-testing) which meet their needs directly when they want to abstract over the different implementations.

@RaasAhsan
Copy link

@djspiewak Not sure if you caught it in the PR, but I ended up parameterizing FiberLocal over F and added a specialized constructor for IO in the companion object. That way we're not committing to creating a type class for it today, but if it becomes feasible in the future then we could without much breakage. Though I think we would need to move it into std for that to work. How do you feel about that?

@djspiewak
Copy link
Member

Uh. Hmm. I'll need to think about it a bit. I really don't think it's going to be something we want to abstract over, but I see what you're saying about binary compatibility.

@RaasAhsan
Copy link

RaasAhsan commented Mar 27, 2021

My original thinking was that our definition of FiberLocal is so simple that Monix and ZIO can implement it with a subset of their functionality i.e. all they need is copy-on-fork semantics, which I think they have. Need to double check. FiberRef goes further and allows shared scopes, but in a completely compositional manner that doesn't demand any extra runtime behavior (via Ref)

@oleg-py
Copy link
Contributor

oleg-py commented Mar 28, 2021

Monix and ZIO can implement it with a subset of their functionality i.e. all they need is copy-on-fork semantics, which I think they have

We definitely don't have copy-on-fork in monix. We opted for making user isolate region explicitly to have CE2 laws like fa.start.flatMap(_.join) <-> fa when fa is mutating local context

@kubukoz
Copy link
Member Author

kubukoz commented Apr 4, 2021

Related: #1847

@zarthross
Copy link
Contributor

Is Monix support still a blocker for having an official FiberLocal? Monix maintenance has slowed (last release is 2 years ago), and there isn't even CE3 support in any of the series/* branches. If the other effect types in the ecosystem support copy-on-fork, I don't really see a blocker here?

@armanbilge
Copy link
Member

@zarthross

If the other effect types in the ecosystem support copy-on-fork, I don't really see a blocker here?

I think it's less a question of what implementations do exist and more a question of what implementations could exist. The blocker is that there isn't an argument why one particular semantic should be preferred for an abstraction.

Taking a step-back, what is your use-case? Is it possible that the Local abstraction from Cats MTL could address your needs? See also #3385.

@zarthross
Copy link
Contributor

I do think Cats MTL Local for IOLocal is a good idea and I will likely use it, part of my issue here is when I create the instance of Local. Most of the code I'm dealing with is still in an abstract F[_] and so creating an instance of IOLocal where I want it is less than Ideal. What I'm effectively looking for is a GenFiberLocal.

I suppose I could hack one together up and create one for Local that could be passed down to where i need it.

trait GenMTLLocal[F[_]]{
  def local[A](default: A): F[Local[F, A]]
}

object GenMTLLocal {
  implicit def ioLocal: new GenMTLLocal[IO] {
     def local[A](default: A): IO[Local[IO, A]] = IOLocal(default).map(oxidized.instances.catsMtlEffectLocalForIO)
  } 
}

@zarthross
Copy link
Contributor

I think it's less a question of what implementations do exist and more a question of what implementations could exist. The blocker is that there isn't an argument why one particular semantic should be preferred for an abstraction.

Not every effect monad implements both Sync and Async, but we still have those. Not every effect system would have to have a FiberLocal instance for FiberLocal to be useful. We can have effect types that don't fully implement the cats-effect typeclass eco system and those still be useful within it.

@armanbilge
Copy link
Member

Not every effect monad implements both Sync and Async, but we still have those.

I'm sorry, I think we talked past each other. My point was that we don't create abstractions by selecting the semantics of an existing implementation and simply swapping in an F[_]. Ideally, we thoughtfully design abstractions independently of implementations, giving consideration to laws and orthogonality.

To frame this issue another way: what is the (theoretical) reason that we should choose the copy-on-fork semantic for this abstraction, independent of what implementations are doing?

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

No branches or pull requests

6 participants