Newtype blocking execution contexts

[mpilquist]

I'd really like a newtype of ExecutionContext that indicates the context is to be used for blocking tasks. Additionally, the newtype could provide good default constructors and convenience methods. I'm roughly thinking of something like this:

package cats.effect

import scala.concurrent.ExecutionContext
import java.util.concurrent.{ExecutorService, Executors}

final class Blocker private (context: ExecutionContext) {

  def delay[F[_], A](thunk: => A)(implicit F: Sync[F], cs: ContextShift[F]): F[A] =
    block(F.delay(thunk))

  def block[F[_], A](fa: F[A])(implicit cs: ContextShift[F]): F[A] =
    cs.evalOn(context)(fa)
}

object Blocker {

  def apply[F[_]](implicit F: Sync[F]): Resource[F, Blocker] =
    fromExecutorService(F.delay(Executors.newCachedThreadPool()))

  def fromExecutorService[F[_]](makeExecutorService: F[ExecutorService])(
      implicit F: Sync[F]): Resource[F, Blocker] =
    Resource
      .make(makeExecutorService)(ec => F.delay { ec.shutdownNow(); () })
      .map(es => unsafeFromExecutionContext(ExecutionContext.fromExecutorService(es)))

  def unsafeFromExecutionContext(ec: ExecutionContext): Blocker = new Blocker(ec)
}

I believe this type would help prevent folks from using the wrong execution context for blocking operations. I'm somewhat in favor of not including unsafeFromExecutionContext at all, though I can see how that would be annoying.

It would be really nice if this could get in to the 2.0.0 release. It might change or be removed for 3.0 but that's okay -- I see it as a companion to the current ContextShift type class.

Blocker definitely treads on the ground covered by linebacker, though I think it's better to keep the newtype unaware of the type constructor you might use with it.

Thoughts? If there's interest, I can prepare a PR.

[djspiewak]

We use this technique in quasar and it works wonderfully. I'd be in favor, but obviously the hour is very very late on 2.0. I'm okay sneaking it in since it's purely additive, but I'd like to hear from more. (hailing @ChristopherDavenport in particular)

[gvolpe]

We used a similar technique at work too. Less chances to make a mistake like passing global (happens all the time!).

I think it's a pretty small addition in terms of lines of code but immensely useful, having it in CE-2.0.0 would be amazing.

[kubukoz]

+1, this will make APIs like fs2's files more explicit about the type of EC expected.

[SystemFw]

I am pretty strongly in favour :)

[lorandszakacs]

Handcrafted this in all production apps that use cats-effect, so it would be really useful to just ship directly in cats-effect 😄

[ChristopherDavenport]

Sounds effectively equivalent to linebacker which was rejected due to doing more than others wanted. I think delay is an overreach and people should use the abilities rather than depend on convenience functions. This was suggested a while ago and explicitly rejected, but I still think it makes sense.

[mpilquist]

I strongly disagree on delay -- it makes it really easy to see where blocking is not occurring. Rather than searching for every instance of F.delay and ensuring each is wrapped in an eval, you can ensure there are no calls to F.delay at all. I'd be okay with renaming it to something else though.

Re: linebacker, I agree though note Blocker is not tied to a type constructor and linebacker does other stuff too (e.g. DualContext, Quarterback). Further, it's another dependency so it's not used in fs2, and that has a network effect.