Cats-effect: `deferFuture` either on `Async` or the `Async` package object.

Created on 6 May 2018  Â·  17Comments  Â·  Source: typelevel/cats-effect

Current interop with future is:

def fromFuture[A](iof: IO[Future[A]]): IO[A] =
    iof.flatMap { f =>
      IO.async { cb =>

    }

I propose something like: (Edited)

object Async {
//...
def deferFuture[F[_], A](f: => Future[A])(implicit F: Async[F]) = {
  F.async {
     f.onComplete(r => cb(r match {
          case Success(a) => Right(a)
          case Failure(e) => Left(e)
        }))(immediate)
      }
  } 
}

Reasons:

  • This is a common operation for people.
  • (Minor reason) fromFuture is 3 extra allocations and evals used right: Pure + Suspend (defer) + Bind + Async. This would only be Async.
  • fromFuture allows you to construct non-RT values with IO.pure or whatever. While you can go wrong with deferFuture at least it is explicitly lazy.

Most helpful comment

We are going in the direction of making Async completely oblivious to side effecting types (e.g removing ExecutionContext and shift from it), so I'm :-1: of having it in the trait.

I kinda like having it on IO only to minimise the surface of the "FFI" with side-effects, relegating the use of => to capture side effects to the "special" constructors like delay.

Otoh, I don't feel the pain since I don't use Future, and it's been pointed out to me that it's quite inconvenient to use fromFuture if you use a type that isn't IO, so overall I won't oppose having it in the companion object of Async, if there's enough demand for it.

All 17 comments

I edited the proposal to have it, like shift, a convenience method in the companion object. It doesn't hurt to have it just be a convenience method, without removing IO.fromFuture.

78 's thread points to the fact that => isn't exactly capturing effects, which is fair, but both fromFuture and deferFuture can be misused, just fromFuture has _more_ ways to misuse it with IO.pure(fut) (where that fut is performing side effects).

Worst case and enough people disagree, then an alleycats-like module for cats-effect is where this can live.

I like it. And I actually like it on the trait because many Task implementations might be close to () => Future[A] and they could optimize this method.

While we are at it, it is not clear to me why there is not F[Future[A]] => F[A] on the trait, object Async or the syntax.

@johnynek having it on the trait means having to define laws for it, which is where it being a convenience method wins out in my mind. That's all. I'm sure we can write laws for this without much issue.

I think they are pretty natural generalizations of the delay laws, but maybe you are right that the opportunity for optimization is not great enough to merit the added complexity.

We are going in the direction of making Async completely oblivious to side effecting types (e.g removing ExecutionContext and shift from it), so I'm :-1: of having it in the trait.

I kinda like having it on IO only to minimise the surface of the "FFI" with side-effects, relegating the use of => to capture side effects to the "special" constructors like delay.

Otoh, I don't feel the pain since I don't use Future, and it's been pointed out to me that it's quite inconvenient to use fromFuture if you use a type that isn't IO, so overall I won't oppose having it in the companion object of Async, if there's enough demand for it.

Maybe we could have something like FutureUtils somewhere, with functions allowing us to do this?

Not all effect types are going to have a particularly good fromFuture implementation. All will have a functional one, but not all will have a good one. Note that the implementation of IO.fromFuture doesn't actually match your proposed implementation, and for good reason. I'm definitely :-1: on this. You can always use IO.fromFuture(f).to[F], where F is your effect type.

Also, we decided quite a while ago to only use => A as a means of capturing "effectful computation of type A" in defer and suspend. If we were to add this function, it would need to have signature F[Future[A]] => F[A].

but not all will have a good one

What do you mean?

You can always use IO.fromFuture(f).to[F], where F is your effect type.

One of the reasons (although minor) for this proposal was avoiding all the allocations with that.

If we were to add this function, it would need to have signature F[Future[A]] => F[A].

Good enough for me. I just don't want to go through IO every time I have to convert a Future to F, e.g. Task :) and currently that's IO.fromFuture(IO(f)).to[F]. Async.deferFuture[F](f) would be a bit better.

Let’s have the safe variant. Users will reimplement it again and again if not and some will get it wrong. No new dependencies no reason not to add it.

Let’s not let our dislike of Future cause needless friction for our users.

The implementation looks more like this:

def fromFuture[F[_], A](f: F[Future[A]])(implicit F: Async[F]): F[A] = {
  f.flatMap { future =>
    // Optimization for already completed futures because "F.async" is expensive
    future.value match {
      case Some(value) => 
        F.fromTry(value)        
      case None => 
        F.async { cb => 
          future.onComplete(r => cb(r.toEither))(immediate)
        }
    }
  }
}

TBH I don't see much potential to introduce more F-specific optimizations. Even IO's current implementation is less optimal than this.

So I'm 👎 on adding it to the type class. Adding it as an utility to the companion object might make sense.

I'm also for adding it to the companion object instead of the typeclass itself.

@kubukoz

One of the reasons (although minor) for this proposal was avoiding all the allocations with that.

If you're in a situation where you're sensitive to two extra allocations at the effect construction boundary (which is effectively what this is), then you shouldn't be using Future. Or IO. Or Async.

@alexandru Your implementation doesn't thread-shift out of the Future pool. That's sort of a subjective design choice, but I've seen the lack of such shifting cause significant and surprising fairness bugs in the past, so I would be hesitant to have an implementation which lacks it.

@johnynek

Let’s not let our dislike of Future cause needless friction for our users.

@LukaJCB

I'm also for adding it to the companion object instead of the typeclass itself.

Your proposal:

FutureUtils.deferFuture(F.delay(fut))

My counter-proposal:

IO.fromFuture(IO(fut)).to[F]

Even on concision alone, the API we have today is preferable to the proposed addition. API surface area is sufficient reason to not include this. It really doesn't have anything to do with disliking Future; IO has a perfectly good fromFuture function and there is no reason – including syntactic convenience! – not to simply point users in that direction.

Your proposal:
FutureUtils.deferFuture(F.delay(fut))

I believe that was supposed to be Async.deferFuture(F.delay(fut))

I'm just closing this for the sake of not being the culprit of creating this sort of bike shedding. This is unimportant. Someone else can reopen it.

hello, I've implemented fromFuture on the Async companion object
I think it's a useful feature
could you guys please have a look if it's something you would find acceptable?

https://github.com/typelevel/cats-effect/pull/453

Was this page helpful?
0 / 5 - 0 ratings