Skip to content
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

Severe memory consumption when using *> operator #401

Closed
dmgcodevil opened this issue Oct 24, 2018 · 11 comments
Closed

Severe memory consumption when using *> operator #401

dmgcodevil opened this issue Oct 24, 2018 · 11 comments

Comments

@dmgcodevil
Copy link

dmgcodevil commented Oct 24, 2018

I'm new to cats-effect and decided to develop a simple app which runs an infinite loop that receives events and performs some side effects. During testing I noticed that performance decreased drastically after 5 millions messages were processed. I started investigation and figured out that the problem is related to usage of *> operator.
From official documentation
the *> operator is defined in Cats and you can treat it as an alias for lh.flatMap(_ => rh)

I created to methods: one uses flatMap to chain effects and another *>

def testFast(counter: Long): IO[Unit] = {
    IO.suspend {
      if(counter == 0) IO.unit
      else IO.pure(counter).flatMap(c => IO(println(c)).flatMap(_ => testFast(c - 1)))
    }
  }

Result:
counter = 10 000 000 total time 33597 ms
Memory consumption is fine, no GS spikes, see visualVM screenshot

Ok, now the same method but flat map has replaced with *>

def testSlow(counter: Long): IO[Unit] = {
    IO.suspend {
      if(counter == 0) IO.unit
      else IO.pure(counter).flatMap(c => IO(println(c)) *> testSlow(c - 1))
    }
  }

Result:
counter = 10 000 000 total time 62128 ms
So if fa *> fb is just a syntax sugar for fa.flatMap( _ => fb) I was expecting to get the same performance but the variant with *> is 2 times slower.

After 13-14m cycles the application has frozen almost completely.

The first thing I noticed was a constantly growing heap and periodic GC spikes, see the screenshot

I created heap dump and analyzed it using MAT and here what I found:

Class Name        |    Objects | Shallow Heap |    Retained Heap
-----------------------------------------------------------------
cats.effect.IO$Map| 25,514,513 |  612,348,312 | >= 2,449,393,256
-----------------------------------------------------------------

histogram

25m Map object of a total Retained Heap = 2GB

Then using Immediate Dominator feature I found this:

Class Name                                    | Shallow Heap | Retained Heap | Percentage
------------------------------------------------------------------------------------------
cats.effect.internals.ArrayStack @ 0x6c6bba340|           32 | 3,134,640,232 |     99.93%
------------------------------------------------------------------------------------------

dominator tree

99% of total Heap is occupied by cats.effect.internals.ArrayStack

Is it a problem with *> or I'm using it wrong ?
I was looking into the source code but I failed to find a reason of such memory consumption.

Could someone assist please ?

@mpilquist
Copy link
Member

mpilquist commented Oct 24, 2018

Try using >> instead of *> -- at first read, it sounds like a case of unguarded non-tail recursion. By using >>, you'll get monadic tail recursion.

I'd also remove the IO.suspend call -- I don't see an obvious reason it needs to be there.

@djspiewak
Copy link
Member

@mpilquist It's worth noting that I can confirm this exact same issue with, what appears to be, the same root cause. Also worth noting that fs2 itself uses *> internally in several functions, which can mean under certain circumstances that this scenario arises outside user control.

To be clear, it appears that use of *> doesn't need to be in a hot path or loop or anything intuitive to trigger this outcome. Any use of it whatsoever in an infinite bind chain (i.e. the application itself) will defeat the trampolining and cause indefinite memory retention.

@mpilquist
Copy link
Member

mpilquist commented Nov 21, 2018

👍 The existing fs2 uses should all be safe but if you see any suspect cases please let me know. OTOH, I wouldn't be against a policy of "always favor >> over *>" in fs2 since it's so easy to make this mistake.

@djspiewak
Copy link
Member

@mpilquist From what I can tell, there are no safe uses, unfortunately. I can add/remove a *> in quasar which sequences a println that happens once at application startup, where the right hand side is effectively the rest of the application, and that alone has a massive impact on memory retention.

@SystemFw
Copy link
Contributor

in fs2 I'd just go with "use >>" , and I'm wondering if it should be an FAQ in cats/cats-effect as well

@mpilquist
Copy link
Member

Oh interesting, so that's different than the non-tail monadic recursion issue. You're saying that ioA *> ioB has the side effect of keeping ioA and ioB on heap until the ioB expression completes b/c the strictness of params results in ioB being reachable/rooted in the object graph?

Is there a potential fix in cats-effect where we somehow null out references to ioA and ioB?

@djspiewak
Copy link
Member

djspiewak commented Nov 21, 2018

@mpilquist I haven't been able to track it down to that degree of precision, but that does appear to be the issue, approximately. Here are some heap traces (pointers go "up", from subsequent lines to previous ones):

Altogether, there are 184 instances of Slice on this particular heap, which cumulatively eat up almost 4 GB of space. Each Slice is retained within a Chunk.singleton, and there is no concurrency whatsoever (the queue you see well down in the trace is a synchronousNoneTerminated). So this is one Stream holding onto every value which passes through it.

The relevant thing to see here is it's all the same instance of ArrayStack, but if you trace through to the other side of the stack, you'll find distinct instances of IO$Map or IO$Bind, indicating that the stack entries are simply not being evicted, which is broadly the issue described by OP. Removing *> usage at the start of the application (before the stream which contains Slices is even created) ameliorates the issue massively, though there's still a memory leak somewhere in this code path that I haven't tracked down (possibly related to the *> usage in fs2 itself).

@djspiewak
Copy link
Member

Also all the code involved is open-source, so I can just push a branch and reproduction instructions if you want, but it's anything but minimal. The system as a whole is extremely complex, and I haven't been able to get it to happen on anything smaller.

@dmgcodevil
Copy link
Author

Herein some insides:

*> - allias for productR

Method chain:

def productR[A, B](fa: F[A])(fb: F[B]): F[B] =
    map2(fa, fb)((_, b) => b)

 def map2[A, B, Z](fa: F[A], fb: F[B])(f: (A, B) => Z): F[Z] =
    map(product(fa, fb))(f.tupled)

   def product[A, B](fa: F[A], fb: F[B]): F[(A, B)] =
    flatMap(fa)(a => map(fb)(b => (a, b)))

@mpilquist
Copy link
Member

Note the following program does not leak memory:

@ def loop: IO[Unit] = IO.unit >> loop
defined function loop

@ (IO.unit *> loop).unsafeRunSync

barambani added a commit to laserdisc-io/laserdisc that referenced this issue Dec 31, 2018
* Workaround for memory issue of `*>`. See typelevel/cats-effect#401
* Monad for Applicative and FlatMap
@djspiewak
Copy link
Member

Is this still a thing? I haven't seen this in a while, but then I've also been careful to use >>.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants