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

Scalaz 8 IO pull request! #1519

Merged
merged 17 commits into from Jan 2, 2018

Conversation

@jdegoes
Member

jdegoes commented Nov 16, 2017

This is a WIP pull request for the Scalaz 8 IO data type and JVM-based runtime system. This is intended to be a wholesale replacement for Scalaz 7 IO and Task.

This pull request includes the following components:

  • scalaz.effect.IO — The core IO data type, which models synchronous and asynchronous effects, with a concurrency model built on fibers and primitives and semantics similar to Haskell's Async package.
  • scalaz.effect.RTS — The runtime system for the JVM, which is capable of effectfully and efficiently interpreting an IO program.
  • scalaz.effect.SafeApp — The entry point for pure functional programs, which is integrated into the runtime system.
  • scalaz.effect.IORef — A mutable reference similar to Haskell's IORef data type.
  • scalaz.effect.MVar — A concurrent variable similar to Haskell's MVar data type.
  • Miscellaneous other utility data types, such as BracketResult, AsyncReturn, and so on.

In addition, a new benchmarks project has been added with competitive benchmarks (Scala Future, cats-effect IO, and Monix Task), which are quite useful for auditing and optimizing the performance of Scalaz 8 IO.

The following work remains outstanding:

  • Type Class Hierarchy — Some work has been done formalizing algebraic and operational laws in a type class hierarchy (MonadFork, Catchable, etc.), but there is more work to do in fleshing out the design.
  • Unit Tests — The JVM runtime system for IO has a growing test suite but many more tests are required, especially for concurrent primitives. Bugs are expected, but at this stage, most should be straightforward to fix. MVar has no test suite and should be considered a sketch.
  • Performance — Performance is very good in most cases, but now that the design is settled, optimization can yield additional improvements, especially in edge cases. Some code has not been optimized for several major iterations and several scalac compiler versions, so additional low-hanging fruit may exist.
  • Concurrency — The current cooperative yielding model is too primitive and thread pool perfunctory, with inadequate control for those needing more powerful knobs for thread shifting. Both of these will be greatly improved in a newer design.
  • Javascript RTS — There currently exists no Javascript runtime system, though almost identical semantics have already been implemented in PureScript so there is an existence proof.
  • Benchmarks — Additional benchmarks would be useful to cover mixed load and purely asynchronous load use cases, as well as benchmarks for MVar when the implementation has been tested and stabilized.
  • Code Cleanup — The runtime system implementation is intended to be low-level, and some duplication is necessary to minimize heap allocations. However, the code can benefit from better organization and some strategic simplifications.

While there is more work that can and must be done, the code has reached a sufficient level of completeness and maturity that it can benefit from outside feedback and collaborators. So with the goal of encouraging this feedback and opening the door to third-party contributions, I am quite happy to present this WIP pull request for Scalaz 8 IO!

Thanks to all those whose feedback and work in other projects inspired and enabled my work here, including Vincent Marquez for the encouragement to work on this project, Alex Nedelscu for the inspiration to work on performance, Daniel Spiewak for his work on a type class hierarchy for effects, and many others for feedback around error handling.

Please dig in, break stuff, and provide whatever feedback or contributions you are inspired to provide!

@rabbitonweb

This comment has been minimized.

Show comment
Hide comment
@rabbitonweb

rabbitonweb Nov 16, 2017

Member

Finally it landed! Now what? :)

Member

rabbitonweb commented Nov 16, 2017

Finally it landed! Now what? :)

@emilypi

This comment has been minimized.

Show comment
Hide comment
@emilypi

emilypi Nov 16, 2017

Member

SafeApp was a pretty surprising and welcome abstraction. Looks great so far.

Member

emilypi commented Nov 16, 2017

SafeApp was a pretty surprising and welcome abstraction. Looks great so far.

@edmundnoble

Looking very nice overall. Curious to see what the new thread/fiber management scheme looks like.

Show outdated Hide outdated effect/README.md Outdated
Show outdated Hide outdated effect/shared/src/main/scala/scalaz/effect/IO.scala Outdated
* An integer that identifies the term in the `IO` sum type to which this
* instance belongs (e.g. `IO.Tags.Point`).
*/
def tag: Int

This comment has been minimized.

@edmundnoble

edmundnoble Nov 16, 2017

Contributor

Should this be public?

@edmundnoble

edmundnoble Nov 16, 2017

Contributor

Should this be public?

This comment has been minimized.

@jdegoes

jdegoes Nov 17, 2017

Member

I could go either way on this. There's no unsafety in this API, but there's also no utility in the API unless you're going to write your own interpreter.

@jdegoes

jdegoes Nov 17, 2017

Member

I could go either way on this. There's no unsafety in this API, but there's also no utility in the API unless you're going to write your own interpreter.

}
object IO extends IOInstances {
object Tags {

This comment has been minimized.

@pedrofurla

pedrofurla Nov 17, 2017

in light of @edmundnoble 's question on https://github.com/scalaz/scalaz/pull/1519/files#r151562686 I wonder if these should be exposed too.

@pedrofurla

pedrofurla Nov 17, 2017

in light of @edmundnoble 's question on https://github.com/scalaz/scalaz/pull/1519/files#r151562686 I wonder if these should be exposed too.

This comment has been minimized.

@jdegoes

jdegoes Nov 17, 2017

Member

Yes, I think if we hide tag we should hide all of these, too.

@jdegoes

jdegoes Nov 17, 2017

Member

Yes, I think if we hide tag we should hide all of these, too.

@puffnfresh

This comment has been minimized.

Show comment
Hide comment
@puffnfresh

puffnfresh Nov 18, 2017

Member

How hard would this be to backport to Scalaz 7? Can it exist as a library?

Member

puffnfresh commented Nov 18, 2017

How hard would this be to backport to Scalaz 7? Can it exist as a library?

}
if (state.compareAndSet(oldState, newState)) loop = false
}

This comment has been minimized.

@TomasMikula

TomasMikula Nov 18, 2017

Member

Scala does have do while loop. 😉
Here and in several other places below var loop can be replaced with a do while loop.

@TomasMikula

TomasMikula Nov 18, 2017

Member

Scala does have do while loop. 😉
Here and in several other places below var loop can be replaced with a do while loop.

@TomasMikula

This comment has been minimized.

Show comment
Hide comment
@TomasMikula

TomasMikula Nov 18, 2017

Member

Looking great! Also 👍 for the documentation!

Btw., how do you reconcile having a data structure for I/O with your opinion that Data Structures Are Antithetical to Functional Programming? 😉

Member

TomasMikula commented Nov 18, 2017

Looking great! Also 👍 for the documentation!

Btw., how do you reconcile having a data structure for I/O with your opinion that Data Structures Are Antithetical to Functional Programming? 😉

@jbgi

Looks really nice! Hope I'll find time to better understand all of it.

Show outdated Hide outdated effect/jvm/src/main/scala/scalaz/effect/console.scala Outdated
Show outdated Hide outdated effect/shared/src/main/scala/scalaz/effect/IO.scala Outdated
Show outdated Hide outdated effect/jvm/src/main/scala/scalaz/effect/RTS.scala Outdated
/**
* Flattens a nested action.
*/
final def flatten[A](io: IO[IO[A]]): IO[A] = io.flatMap(a => a)

This comment has been minimized.

@notxcain

notxcain Nov 19, 2017

Why there is no flatten method on IO[A]? Given ev: A <:< IO[B] it's just a flatMap(ev). Same question about absolve.

@notxcain

notxcain Nov 19, 2017

Why there is no flatten method on IO[A]? Given ev: A <:< IO[B] it's just a flatMap(ev). Same question about absolve.

This comment has been minimized.

@jdegoes

jdegoes Nov 19, 2017

Member

Because this should be Liskov but we do not have such a type class yet in Scalaz 8. 😄 I will definitely add both when we get Liskov.

@jdegoes

jdegoes Nov 19, 2017

Member

Because this should be Liskov but we do not have such a type class yet in Scalaz 8. 😄 I will definitely add both when we get Liskov.

This comment has been minimized.

@TomasMikula

TomasMikula Nov 19, 2017

Member

This comment has been minimized.

@notxcain

notxcain Nov 19, 2017

Ah, I see. Ok then :)

@notxcain

notxcain Nov 19, 2017

Ah, I see. Ok then :)

@jdegoes

This comment has been minimized.

Show comment
Hide comment
@jdegoes

jdegoes Nov 19, 2017

Member

Btw., how do you reconcile having a data structure for I/O with your opinion that Data Structures Are Antithetical to Functional Programming? 😉

The grand conclusion of many of my articles is to do the exact opposite of the best practice. Because the languages we use are so terrible. IO and Scala are no exception. 😉

Member

jdegoes commented Nov 19, 2017

Btw., how do you reconcile having a data structure for I/O with your opinion that Data Structures Are Antithetical to Functional Programming? 😉

The grand conclusion of many of my articles is to do the exact opposite of the best practice. Because the languages we use are so terrible. IO and Scala are no exception. 😉

@jdegoes

This comment has been minimized.

Show comment
Hide comment
@jdegoes

jdegoes Nov 19, 2017

Member

Last minute changes I made to the code before the pull request actually de-optimized it (in most cases). It's nearly back to being faster than it's ever been, just a bit more work to do...

All the feedback has been incorporated. I'll cleanup a few things then probably just merge this as-is with new tickets for the outstanding work (unless folks have any more suggestions).

Member

jdegoes commented Nov 19, 2017

Last minute changes I made to the code before the pull request actually de-optimized it (in most cases). It's nearly back to being faster than it's ever been, just a bit more work to do...

All the feedback has been incorporated. I'll cleanup a few things then probably just merge this as-is with new tickets for the outstanding work (unless folks have any more suggestions).

@jdegoes

This comment has been minimized.

Show comment
Hide comment
@jdegoes

jdegoes Nov 19, 2017

Member

@puffnfresh Backport would be pretty easy. Standalone library is easy too, but I will have Scalaz 8 builds running in a week or so, so people can pull in 8 just for IO with ease (then define, e.g. Scalaz 7 or Cats Effect type class instances for it).

Member

jdegoes commented Nov 19, 2017

@puffnfresh Backport would be pretty easy. Standalone library is easy too, but I will have Scalaz 8 builds running in a week or so, so people can pull in 8 just for IO with ease (then define, e.g. Scalaz 7 or Cats Effect type class instances for it).

@jbgi

This comment has been minimized.

Show comment
Hide comment
@jbgi

jbgi Nov 19, 2017

Member

@jdegoes see also some minor fixes in jdegoes#1 (not sure relevant anymore after last commits)

Member

jbgi commented Nov 19, 2017

@jdegoes see also some minor fixes in jdegoes#1 (not sure relevant anymore after last commits)

@japgolly

This comment has been minimized.

Show comment
Hide comment
@japgolly

japgolly Nov 19, 2017

Contributor

I'm curious: will this constant fold, being in a trait? If yes, have you verified so?

Contributor

japgolly commented on effect/jvm/src/main/scala/scalaz/effect/RTS.scala in 58e6d21 Nov 19, 2017

I'm curious: will this constant fold, being in a trait? If yes, have you verified so?

This comment has been minimized.

Show comment
Hide comment
@jdegoes

jdegoes Nov 20, 2017

Member

It will definitely not constant fold. In fact, Scala will insist on accessing it via invokevirtual. 😢

However, it doesn't matter since I copy it to the stack before I use it anywhere.

Member

jdegoes replied Nov 20, 2017

It will definitely not constant fold. In fact, Scala will insist on accessing it via invokevirtual. 😢

However, it doesn't matter since I copy it to the stack before I use it anywhere.

This comment has been minimized.

Show comment
Hide comment
@japgolly

japgolly Nov 20, 2017

Contributor

Ok cool, thanks for the info, that's good to know. 👍

Contributor

japgolly replied Nov 20, 2017

Ok cool, thanks for the info, that's good to know. 👍

You can surface failures with `attempt`, which takes an `IO[A]` and produces an `IO[Throwable \/ A]`:
```scala
val result = openFile("data.json").attempt.map {

This comment has been minimized.

@scr

scr Nov 19, 2017

Where is this awesome-sauce? I've been curious about how one could have truly non-blocking IO, which called back to the rts rather than having to waste a thread with something like Files.readAllBytes(...). Java 8 has some sort of stuff in java.nio, Selector, SelectableChannel, AsynchronousFileChannel, e,g. that seems to offer some non-blocking facilities that look quite painful to manage as a client and perfect for rolling into something like this so clients can just program naturally without worrying about the nitty gritty of selecting, registering keys, reading more bytes when ready, etc. When I first saw this readme a few days ago I was hopeful that truly non-blocking yet sensibly concurrent/parallel file operations existed, but I guess they're just a concept/WIP? If not please point me to where I can look at how they're hooked up!

@scr

scr Nov 19, 2017

Where is this awesome-sauce? I've been curious about how one could have truly non-blocking IO, which called back to the rts rather than having to waste a thread with something like Files.readAllBytes(...). Java 8 has some sort of stuff in java.nio, Selector, SelectableChannel, AsynchronousFileChannel, e,g. that seems to offer some non-blocking facilities that look quite painful to manage as a client and perfect for rolling into something like this so clients can just program naturally without worrying about the nitty gritty of selecting, registering keys, reading more bytes when ready, etc. When I first saw this readme a few days ago I was hopeful that truly non-blocking yet sensibly concurrent/parallel file operations existed, but I guess they're just a concept/WIP? If not please point me to where I can look at how they're hooked up!

This comment has been minimized.

@jdegoes

jdegoes Nov 20, 2017

Member

It does not exist yet, but will most definitely exist soon. 😄

The first step is scalaz-nio.

@jdegoes

jdegoes Nov 20, 2017

Member

It does not exist yet, but will most definitely exist soon. 😄

The first step is scalaz-nio.

This comment has been minimized.

@scr

scr Nov 20, 2017

Ok, so I played with how one might read a file (hopefully some file manipulation can be rolled into the library), but I can't quite get it the final mile (i.e. so that each chunk read is done from the RTS so that cancellation or fiber-timeslicing can occur) - sorry, but I just can't my head around how to convert impure code i.e. where making the same call could have different results (bytes read) into pure IO/effect code and how to keep making that call like a do while in the IO RTS… Any advice/insight appreciated!

https://gist.github.com/scr/9d9fd533f02f7dcf4c4029ab55901412

@scr

scr Nov 20, 2017

Ok, so I played with how one might read a file (hopefully some file manipulation can be rolled into the library), but I can't quite get it the final mile (i.e. so that each chunk read is done from the RTS so that cancellation or fiber-timeslicing can occur) - sorry, but I just can't my head around how to convert impure code i.e. where making the same call could have different results (bytes read) into pure IO/effect code and how to keep making that call like a do while in the IO RTS… Any advice/insight appreciated!

https://gist.github.com/scr/9d9fd533f02f7dcf4c4029ab55901412

This comment has been minimized.

@scr

scr Nov 20, 2017

I see there's a forever and retry but both of those work on failures - it would seem that a tri-state is needed, or Throwable \/ Option[T] with something like a .doWhile or .doUntil operator that calls the code until it receives either the throwable or an empty/None result (or nonEmpty/Some result if doUntil.

@scr

scr Nov 20, 2017

I see there's a forever and retry but both of those work on failures - it would seem that a tri-state is needed, or Throwable \/ Option[T] with something like a .doWhile or .doUntil operator that calls the code until it receives either the throwable or an empty/None result (or nonEmpty/Some result if doUntil.

This comment has been minimized.

@scr

scr Nov 20, 2017

or some sort of IO.trampoline thing?

@scr

scr Nov 20, 2017

or some sort of IO.trampoline thing?

This comment has been minimized.

@scr

scr Nov 21, 2017

Ok… got it working with existing primitives (thanks @jdegoes for nudge to try IO.flatten); updated gist to show how IO.flatten of the IO.async[IO[A]] lets you chain File work indefinitely to read the entire file in and give chances for cooperation with this Fiber stuff! So cool!

@scr

scr Nov 21, 2017

Ok… got it working with existing primitives (thanks @jdegoes for nudge to try IO.flatten); updated gist to show how IO.flatten of the IO.async[IO[A]] lets you chain File work indefinitely to read the entire file in and give chances for cooperation with this Fiber stuff! So cool!

@mielientiev

This comment has been minimized.

Show comment
Hide comment
@mielientiev

mielientiev Nov 29, 2017

Hi @jdegoes
Maybe I miss something, but I wonder is it possible to "convert" scala future to scalaz 8 IO?

(I mean if I already have a legacy method that returns a future and I want to wrap it to IO)

mielientiev commented Nov 29, 2017

Hi @jdegoes
Maybe I miss something, but I wonder is it possible to "convert" scala future to scalaz 8 IO?

(I mean if I already have a legacy method that returns a future and I want to wrap it to IO)

@mielientiev

This comment has been minimized.

Show comment
Hide comment
@mielientiev

mielientiev Nov 29, 2017

reply to my previous message, I believe it's very easy to achieve like:

def funcWithFuture: Future[Int] = ???
val result: IO[Int] = IO.async { cb =>
    funcWithFuture.onComplete {
        case Success(a) => cb(\/-(a))
        case Failure(t) => cb(-\/(t))
    }
}

mielientiev commented Nov 29, 2017

reply to my previous message, I believe it's very easy to achieve like:

def funcWithFuture: Future[Int] = ???
val result: IO[Int] = IO.async { cb =>
    funcWithFuture.onComplete {
        case Success(a) => cb(\/-(a))
        case Failure(t) => cb(-\/(t))
    }
}
@jdegoes

This comment has been minimized.

Show comment
Hide comment
@jdegoes

jdegoes Nov 29, 2017

Member

@mielientiev Yes, it's very easy (as the code you have), but this functionality will have to be placed in a third-party library. I can help write that after I get this PR in.

Member

jdegoes commented Nov 29, 2017

@mielientiev Yes, it's very easy (as the code you have), but this functionality will have to be placed in a third-party library. I can help write that after I get this PR in.

@mielientiev

This comment has been minimized.

Show comment
Hide comment
@mielientiev

mielientiev Nov 29, 2017

@jdegoes Also how can we do vice versa? (IO -> Future), I found only sync unsafePerform, but I didn't find any way to run it in an async fashion? Can you please point me? ;)

mielientiev commented Nov 29, 2017

@jdegoes Also how can we do vice versa? (IO -> Future), I found only sync unsafePerform, but I didn't find any way to run it in an async fashion? Can you please point me? ;)

@jdegoes

This comment has been minimized.

Show comment
Hide comment
@jdegoes

jdegoes Nov 29, 2017

Member

@mielientiev I can make that function public in my next update (it's buried in RTS).

Member

jdegoes commented Nov 29, 2017

@mielientiev I can make that function public in my next update (it's buried in RTS).

@mielientiev

This comment has been minimized.

Show comment
Hide comment
@mielientiev

mielientiev Nov 29, 2017

@jdegoes it will be great! Thanks

mielientiev commented Nov 29, 2017

@jdegoes it will be great! Thanks

@mielientiev

This comment has been minimized.

Show comment
Hide comment
@mielientiev

mielientiev Dec 2, 2017

@jdegoes by the way, just curious why AsyncEffect and accordingly IO.async uses (Throwable \/ A => Unit) and not a Try[A] => Unit ?

mielientiev commented Dec 2, 2017

@jdegoes by the way, just curious why AsyncEffect and accordingly IO.async uses (Throwable \/ A => Unit) and not a Try[A] => Unit ?

@eleaar

This comment has been minimized.

Show comment
Hide comment
@eleaar

eleaar Dec 2, 2017

@jdegoes @mielientiev The transformation from IO -> Future could be easily doable with just a few changes to the existing code, by separating the actual fiber runtime from the interpreter itself.

I've spiked an example here: jdegoes#3.
What do you think of it?

eleaar commented Dec 2, 2017

@jdegoes @mielientiev The transformation from IO -> Future could be easily doable with just a few changes to the existing code, by separating the actual fiber runtime from the interpreter itself.

I've spiked an example here: jdegoes#3.
What do you think of it?

@jdegoes

This comment has been minimized.

Show comment
Hide comment
@jdegoes

jdegoes Dec 4, 2017

Member

@mielientiev There is no Try, or rather, Try[A] is a synonym for Throwable \/ A. I will delete the type alias since it's confused people. I just did it to save typing.

@eleaar I like it and I have had some time to digest. I need to figure out one more thing before I implement this (or something like it).

Member

jdegoes commented Dec 4, 2017

@mielientiev There is no Try, or rather, Try[A] is a synonym for Throwable \/ A. I will delete the type alias since it's confused people. I just did it to save typing.

@eleaar I like it and I have had some time to digest. I need to figure out one more thing before I implement this (or something like it).

@Baccata

This comment has been minimized.

Show comment
Hide comment
@Baccata

Baccata Dec 18, 2017

I've started trying it to get acquainted with the API. It looks great, however it looks like fibers prevent the process from finishing. For instance, a simple program like the following does print as expected, but then jvm just hangs there afterwards :

import scalaz.effect.{ IO, SafeApp }
import scala.concurrent.duration._

object Main extends SafeApp {
  override def run(args: List[String]): IO[Unit] =
    IO.sync(println("Hello world")).timeout(1.seconds)
}

Is this is the expected behavior ?

Baccata commented Dec 18, 2017

I've started trying it to get acquainted with the API. It looks great, however it looks like fibers prevent the process from finishing. For instance, a simple program like the following does print as expected, but then jvm just hangs there afterwards :

import scalaz.effect.{ IO, SafeApp }
import scala.concurrent.duration._

object Main extends SafeApp {
  override def run(args: List[String]): IO[Unit] =
    IO.sync(println("Hello world")).timeout(1.seconds)
}

Is this is the expected behavior ?

@eleaar

This comment has been minimized.

Show comment
Hide comment
@eleaar

eleaar Dec 18, 2017

I've had the same problem when using this branch as a submodule in a larger test project.
The reason is that the underlying ExecutorServices are not being explicitly shut down in the SafeApp. I've ended up doing a:

trait SafeApp2 extends RTS {

  def run(args: List[String]): IO[Unit]

  final def main(args0: Array[String]): Unit = {
   try { 
    unsafePerformIO(run(args0.toList))
   } finally {
    threadPool.shutdown()
    scheduledExecutor.shutdown()
   }
  }
}

Oddly enough, I didn't experience this behaviour when running a SafeApp from within a fork of this branch.

eleaar commented Dec 18, 2017

I've had the same problem when using this branch as a submodule in a larger test project.
The reason is that the underlying ExecutorServices are not being explicitly shut down in the SafeApp. I've ended up doing a:

trait SafeApp2 extends RTS {

  def run(args: List[String]): IO[Unit]

  final def main(args0: Array[String]): Unit = {
   try { 
    unsafePerformIO(run(args0.toList))
   } finally {
    threadPool.shutdown()
    scheduledExecutor.shutdown()
   }
  }
}

Oddly enough, I didn't experience this behaviour when running a SafeApp from within a fork of this branch.

@jdegoes

This comment has been minimized.

Show comment
Hide comment
@jdegoes

jdegoes Dec 18, 2017

Member

@eleaar This is a known issue. It creates threads which hold up exit of the JVM. I'll fix it, for sure.

Member

jdegoes commented Dec 18, 2017

@eleaar This is a known issue. It creates threads which hold up exit of the JVM. I'll fix it, for sure.

@jdegoes

This comment has been minimized.

Show comment
Hide comment
@jdegoes
Member

jdegoes commented Dec 18, 2017

@Baccata ^^^

@mielientiev

This comment has been minimized.

Show comment
Hide comment
@mielientiev

mielientiev Dec 18, 2017

@jdegoes when does this PR would be merged and released?

mielientiev commented Dec 18, 2017

@jdegoes when does this PR would be merged and released?

@jdegoes

This comment has been minimized.

Show comment
Hide comment
@jdegoes

jdegoes Dec 18, 2017

Member

@mielientiev Next week.

Member

jdegoes commented Dec 18, 2017

@mielientiev Next week.

@fwbrasil

This comment has been minimized.

Show comment
Hide comment
@fwbrasil

fwbrasil Dec 24, 2017

@jdegoes it seems that this new IO implementation suffers from the same problem as cat-effect regarding strict evaluation. See typelevel/cats-effect#92. Here's a simplified repro:

def sum(l: List[IO[Int]]): IO[Int] =
  l match {
    case head :: tail => 
      head
        .map(i => sum(tail).map(_ + i))
        .flatMap(identity)
  }

// java.lang.StackOverflowError
sum(List.fill(100000)(IO.now(1)))

fwbrasil commented Dec 24, 2017

@jdegoes it seems that this new IO implementation suffers from the same problem as cat-effect regarding strict evaluation. See typelevel/cats-effect#92. Here's a simplified repro:

def sum(l: List[IO[Int]]): IO[Int] =
  l match {
    case head :: tail => 
      head
        .map(i => sum(tail).map(_ + i))
        .flatMap(identity)
  }

// java.lang.StackOverflowError
sum(List.fill(100000)(IO.now(1)))
@jdegoes

This comment has been minimized.

Show comment
Hide comment
@jdegoes

jdegoes Dec 24, 2017

Member

@fwbrasil Thanks for reporting, and good to see you here!

This limitation is documented in the source code and README and probably won't be fixed. map is guaranteed stack-safe for up to 10k repeated evaluations, but not beyond.

Similarly, io.map(f1).map(f2).map(f3)...map(fn) is by law equivalent to io.map(f1 >>> f2 >>> f3 >>> ... fn), but obviously the latter will stack overflow even earlier than the former and cannot be "fixed".

I'd definitely consider fixing this but I'd like to see a non-contrived example of code people have actually written that triggers the SOE. If anyone reading this has any such example, please share!

Member

jdegoes commented Dec 24, 2017

@fwbrasil Thanks for reporting, and good to see you here!

This limitation is documented in the source code and README and probably won't be fixed. map is guaranteed stack-safe for up to 10k repeated evaluations, but not beyond.

Similarly, io.map(f1).map(f2).map(f3)...map(fn) is by law equivalent to io.map(f1 >>> f2 >>> f3 >>> ... fn), but obviously the latter will stack overflow even earlier than the former and cannot be "fixed".

I'd definitely consider fixing this but I'd like to see a non-contrived example of code people have actually written that triggers the SOE. If anyone reading this has any such example, please share!

@fwbrasil

This comment has been minimized.

Show comment
Hide comment
@fwbrasil

fwbrasil Dec 24, 2017

@jdegoes I find Alexandru's code an appealing example of a real-world scenario. I wouldn't wait for people to stumble across this issue to fix it but it's your call :)

BTW, I've been working on that Arrow implementation we spoke about, but for twitter-util. I hope to have something ready in January

fwbrasil commented Dec 24, 2017

@jdegoes I find Alexandru's code an appealing example of a real-world scenario. I wouldn't wait for people to stumble across this issue to fix it but it's your call :)

BTW, I've been working on that Arrow implementation we spoke about, but for twitter-util. I hope to have something ready in January

@jdegoes

This comment has been minimized.

Show comment
Hide comment
@jdegoes

jdegoes Jan 2, 2018

Member

@fwbrasil This class of examples can only be constructed with composition of point and map, i.e. effect-free code. I am not sure it's a useful example in real-life. However, I will open a ticket and implement a fix if I can preserve most of the performance gains of the current approach (it should be possible).

Also really look forward to seeing the Arrow implementation. ❤️ Keep me posted!

Member

jdegoes commented Jan 2, 2018

@fwbrasil This class of examples can only be constructed with composition of point and map, i.e. effect-free code. I am not sure it's a useful example in real-life. However, I will open a ticket and implement a fix if I can preserve most of the performance gains of the current approach (it should be possible).

Also really look forward to seeing the Arrow implementation. ❤️ Keep me posted!

@jdegoes jdegoes merged commit 1ae6cb3 into scalaz:series/8.0.x Jan 2, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@hrhino hrhino moved this from In Progress to Done in Scalaz 8 May 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment