typelevel / cats-effect

The pure asynchronous runtime for Scala
https://typelevel.org/cats-effect/
Apache License 2.0
2.03k stars 520 forks source link

optimize `IO.whenA` #4135

Closed Jasper-M closed 1 month ago

Jasper-M commented 2 months ago

I noticed that this simple recursive loop

def loop(i: Long): IO[Unit] =
  IO.whenA(i % 1000_000 == 0)(IO.println(i / 1000_000)).flatMap{ _ =>
    IO.whenA(i < Long.MaxValue)(loop(i + 1))
  }

loop(0).unsafeRunSync()

ends with

1071
1072
1073
java.lang.NegativeArraySizeException: -2147483648
    at cats.effect.ArrayStack.checkAndGrow(ArrayStack.scala:73)
    at cats.effect.ArrayStack.push(ArrayStack.scala:34)
    at cats.effect.IOFiber.runLoop(IOFiber.scala:367)
    at cats.effect.IOFiber.autoCedeR(IOFiber.scala:1423)
    at cats.effect.IOFiber.run(IOFiber.scala:119)
    at cats.effect.unsafe.WorkerThread.run(WorkerThread.scala:743)

While the equivalent if else code seems to keep running without noticeable GC pauses.

armanbilge commented 2 months ago

Interesting, that must be because of the void(...) in the Applicative#whenA implementation? We avoid that b/c we require that the argument is already voided.

https://github.com/typelevel/cats/blob/927a9bb957530b144506e96fc78bff67553858be/core/src/main/scala/cats/Applicative.scala#L263-L264

See also:

Jasper-M commented 2 months ago

We could do that, but this slimmed down version still takes 20 seconds to complete on my machine. And that's in the happy path where it doesn't fail or use a ridiculous amount of memory.

def loop(i: Long): IO[Unit] =
  IO.unit >> IO.whenA(i < 1_100_000_000)(loop(i + 1))
lenguyenthanh commented 2 months ago

Interesting, that must be because of the void(...) in the Applicative#whenA implementation?

Should We do something about IO.void, ex:

   def void: IO[Unit] =
    5 -    map(_ => ())
    6 +    isInstanceOf[IO[Unit]] match {
    7 +      case true => this.asInstanceOf[IO[Unit]]
    8 +      case _ => map(_ => ())
    9 +    }
armanbilge commented 2 months ago

isInstanceOf[IO[Unit]]

@lenguyenthanh Unfortunately this won't work because type parameters are erased at runtime, so it will always be true.

Jasper-M commented 1 month ago

Can you add an override in the Async typeclass implementation for both methods as well? That will allow the optimization to work in polymorphic contexts.

The problem is like @armanbilge pointed out that the typeclass method takes a => F[A] whereas the IO method takes => IO[Unit]. So I don't think you can do any better than if (cond) void(f) else unit in the typeclass. Maybe the method signature in cats-core should be fixed, but that seems hard to do without breaking things.

armanbilge commented 1 month ago

Both because the signature in Cats is definitely wrong

This might be a matter of opinion, see a discussion on this topic in https://github.com/typelevel/cats/pull/4352#discussion_r1028533867.