-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Should Monad extend Defer? #4579
Comments
My gut tells me bad idea, and my default position is to be irritable about anything source breaking. But I've almost talked myself into this after a small spike. |
I think it is a bad idea because strict monads, such as cats.Id would just have to directly evaluate this totally destroying the intent of Defer. Aside from intend, it would fail the current laws because the current law says defer shouldn't evaluate the arg if you just call it. That said, maybe that's a bad law... idk.... but if that doesn't work, I don't see how I think maybe StackSafeMonad could extend Defer in this way. That said, it's not that hard to add this implementation to the StackSafeMonad you are implementing. |
You're right. I didn't have enough compiling to run more tests than MiMa, but it definitely breaks both the spirit and the letter of the law in several cases. A place I've seen this come up repeatedly, and again today, is to avoid the eager instantiation of the fallback value on timeouts. When all people have is |
I like the idea to consider Moreover, I would say that currently |
Yes indeed. This idea is discussed before in #3790 (comment).
Daniel explained recently on Discord:
|
Future has Aside: the stack safety of the current instance depends on the stack safety of its |
@rossabaker, actually, Custom Future implementation (folded, because even a minimal implementation is fairly large): final class LazyFuture[+T](get: () => Future[T]) extends Future[T] {
private lazy val future = get()
override def isCompleted: Boolean = future.isCompleted
override def value: Option[Try[T]] = future.value
override def onComplete[U](f: Try[T] => U)(implicit executor: ExecutionContext): Unit =
future.onComplete(f)
override def transform[S](f: Try[T] => Try[S])(implicit executor: ExecutionContext): Future[S] =
future.transform(f)
override def transformWith[S](f: Try[T] => Future[S])(implicit executor: ExecutionContext): Future[S] =
future.transformWith(f)
override def ready(atMost: Duration)(implicit permit: CanAwait): this.type = {
future.ready(atMost)
this
}
override def result(atMost: Duration)(implicit permit: CanAwait): T =
future.result(atMost)
} Now, the implementation of object FutureDefer extends Defer[Future] {
override def defer[A](fa: => Future[A]): Future[A] =
new LazyFuture[A](() => fa)
} An example of demo code: implicit val executor: ExecutionContext = ExecutionContext.parasitic
var evaluated1 = false
var evaluated2 = false
val deferred = FutureDefer.defer {
evaluated1 = true
Future {
evaluated2 = true
12345
}
}
println("allow some time, just in case...")
Thread.sleep(1000)
println(s"evaluated1=$evaluated1; evaluated2=$evaluated2")
println("now await the future...")
val result = Await.result(deferred, Duration.Inf)
println(s"evaluated1=$evaluated1; evaluated2=$evaluated2")
println(s"result=$result") And here is the output:
So it looks working even with the That said, I am not sure if we can expect from val original = Future {
evaluated2 = true
12345
}
val deferred = FutureDefer.defer {
evaluated1 = true
original
} Apparently, the output is:
But even in that case, |
Actually, I'm inclining to think that val M = Monad[Eval]
println("begin")
M.tailRecM(0) { a =>
println(s"a=$a")
Eval.always(Either.cond(a == 3, "done", a + 1))
}
println("end") Even though we do not ask for the result
I feel it might not be what users would expect in general. But if trait StackSafeMonad[F[_]] extends Monad[F] with Defer[F] {
override def tailRecM[A, B](a: A)(f: A => F[Either[A, B]]): F[B] = {
def loop(aa: A): F[B] =
flatMap(f(aa)) {
case Left(aaa) => loop(aaa)
case Right(b) => pure(b)
}
defer(loop(a))
}
} |
I'm wary of arguments about pure FP that leverage hidden side-effects to show their value. In the case of using I know sometimes people do this, but generally, we shouldn't design for it. That said, FlatMap.tailRecM and Defer.defer (similar to Monoid.combineAll) are making concessions to the fact that we are implementing this code on virtual machines where function evaluations consume a finite resource (stack space), and so by declaring the intent of the code directly we can get a more efficient encoding on those virtual machines. So the law in Defer about not evaluating the argument is really motivated by the desire for O(1) stack usage to do A secondary concern is the ability to short circuit a computation (e.g. foldRight) but that is only about improving the best or average case scenario, it doesn't change the worst case performance. |
I agree with Oscar's sentiment here. The motivating case here is the secondary concern: improving the average case of a timeout, when the fallback value requires an expensive stack trace and is rarely returned. We fake that today with the I still wonder whether |
For some definition of correct :) constructing an exception is not pure, it is side-effectful. Depending on where and when you do it, you will capture a different stack trace. If we decide we don't care about stack traces, then it doesn't matter. I wonder if what we need is a dedicated way to create |
The "unit-flatMap" idiom is used in various places for deferring the creation of a value where no
Defer
instance is available:This is not always the optimal
defer
implementation for a Monad, but it's always a possible one and can be overridden with optimal. It would also be binary compatible to extendMonad
withDefer
with this default implementation. Furthermore, with some rearrangements of instance inheritance, it seems source compatible with Cats-defined instances. Types outside Cats that define separate Monad and Defer instances today would become ambiguous without similar tricks, so it's not entirely source compatible through the ecosystem.Before I go further: good idea or bad idea?
The text was updated successfully, but these errors were encountered: