optimize IO.whenA

[Jasper-M]

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]

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:

• ArrayIndexOutOfBoundsException in ByteStack #3907

[armanbilge]

Shall we add a test case based on your example?

[Jasper-M]

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))

[armanbilge]

Ah yes, d'oh, good point. Probably not worth the strain on CI.

[lenguyenthanh]

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]

isInstanceOf[IO[Unit]]

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

[djspiewak]

This makes quite a bit of sense honestly. Also it raises the question of when and why whenA would actually be desirable…

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.

[Jasper-M]

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.

[djspiewak]

Ooooooh yeah this is exceptionally annoying. Both because the signature in Cats is definitely wrong, but also because we're inconsistent with it and I didn't realize. At any rate, bigger fish than we can fry right now.

[armanbilge]

Both because the signature in Cats is definitely wrong

This might be a matter of opinion, see a discussion on this topic in typelevel/cats#4352 (comment).