search

brettwooldridge / NuProcess

Low-overhead, non-blocking I/O, external Process implementation for Java
Apache License 2.0
710 stars 84 forks source link

Memory consumption increases in a linear manner with the same workload #127

Closed kartoffelsup closed 3 years ago

kartoffelsup commented 3 years ago

Hi,

thank you for creating NuProcess :)

we wanted to use NuProcess to ping many hosts in a scheduled task but we ran into an issue where the memory usage just kept increasing. I've reproduced this with a minimal example below:

grafik

I've setup two test runners in AWS ECS Fargate.

JavaProcessTest.java

NuProcesstest.java

Are we using NuProcess the wrong way or is this a bug?

Any help is appreciated :)

bturner commented 3 years ago

What you're seeing is the illusion of increased memory usage.

The sample app isn't triggering any sort of GC, so until the collector decides to step in (which, so long as it has heap headroom, it won't), garbage is allowed to build up. Pair that with the fact that sample produces quite a lot of garbage (because it allocates a new byte[] each time it wants to copy data out of a ByteBuffer) and you get a nice, steady climb. If you add a System.gc() before you write the sizes after joining all your Futures you'll get a perfectly flat line. I've had that running for the last 3 hours and the memory numbers haven't changed at all. (Edit: Just to be clear, I'm not saying you'd trigger manual GC in production code; you'd rely on the garbage collection handling to do what it needs to, when it needs to. I'm saying for your sample program, for this type of test where you're looking at memory usage from the perspective of trying to find leaks, you need to control GC more explicitly for your numbers to mean anything.)

In practice, if you're going to translate from ByteBuffer to String, you probably want to use something like NuAbstractCharsetHandler to help. That's going to handle split multibyte characters correctly (which the sample code currently wouldn't; of course, if it's only dealing with ASCII output that's not a concern), and it's also going to reuse buffers between callbacks, which reduces allocation churn. (You can, of course, write your own decoding/buffering; I'm just pointing out an existing helper.)

There's a second bug in the sample, just FYI, in how append deals with closed. closed being true does not mean the buffer is empty; you can get data on the final callback (and often do). The current handling means the sample doesn't necessarily process all the output. Replacing the if (!closed) with if (buffer.hasRemaining()) would fix that.

kartoffelsup commented 3 years ago

Hi,

thanks for the explanation and the tips :) I've reproduced what you described, sorry for wasting your time with this non-bug.

image (2)