Memory leaks from GenServer processes

OMG, I have been reading it as map all the time! Editing post.

Enum.each is the right thing to use when you only care about side-effects, it always returns :ok and doesn’t accumulate a list.

https://github.com/elixir-lang/elixir/blob/v1.7.3/lib/elixir/lib/enum.ex#L764

EDIT: Ignore the Enum.each comments, I mistook it for map. My bad.

First commit:

• Not sure why you use Poison. Jason is the newer library and is faster.
• You still have not made your code use string map keys instead of atoms. You only get 1_048_476 atoms (here is the source) and they are never garbage-collected. It’s easy to execute a denial-of-service attack against Elixir code that blindly accepts all atoms thrown at it via external data. I mean this is not actually a major concern in the grand scheme of things (you will probably protect your API behind an authentication wall) but since you are trying to troubleshoot a memory consumption problem I believe it’s worth the effort to modify your code to never use atom JSON keys. I personally also believe your producer/consumer queues hold on to strings for a long time and that’s your problem but hey, why not try everything since you decided to try and tackle the problem?
• Outside of those, I believe the commit mostly covers what @aseigo meant earlier.

Second commit: if you are not interested in return value of Enum.each I suggest the following instead.

Change all these:

Enum.each(data.guilds, fn(guild) -> handle(:GUILD_UPDATE, guild) end)

to these:

Stream.each(data.guilds, fn(guild) -> handle(:GUILD_UPDATE, guild) end)
|> Stream.run

Sure it’s little longer but (1) it does not create a return value and does not pass and modify it between each iteration only for it to eventually be discarded, and (2) communicates the intent of the code better: you are not interested in the results of the loop, you only invoke it for the side effects.

Do this everywhere you do NOT assign anything to the Enum.each calls.

Would it be something like this + this?

no problems; it’s really nice to see people taking on projects like yours with Elixir! best of luck and happy hacking …

I am at work right now so I can’t test all this, but I will definitely try every single suggestion.

Thank you very much for such answer

It looks like there are a number of improvements that could be made so your stages run smoother.

For instance, in Coxir.Struct you are using ets as an in-memory backing store and have these encode/1 and decode/1 functions. Those are getting used a lot and just change a Map into a list to a tuple and back… It looks like you are doing this so you can store it in an ets table which needs tuples .. but .. you could just do:

:ets.insert @table, {id, data_as_a_map}

Then you will have equivalent lookups:

[{_id, data}] = :ets.lookup @table, id

No moving data between different data structures constantly. Whether or not that is causing your memory usage (doubtful), it will certainly relieve some pressure on the GC and make your code faster.

In Coxir.Stage.Middle.handle/2 there are many places where you are using for list comprehensions, but discarding the results. for creates a list out of every single result .. if you do not use those results, the for is storing them in a list for no good reason. What you probably want instead is Enum.each/2 which just calls the function for each entry in the enumerable, IOW: for the side-effects. List comprehensions are there to build lists, not create side effects.

More plainly: for in Elixir is not equivalent to a for-loop in your typical imperative language. Enum.each/2 is closer to loops that do not change external variables, and Enum.reduce/3 is closer to those that do.

There is also interesting code in Consumer like:

handler
|> apply(:handle_event, [event, state])

where you could just do:

handler.handle_event(event, state)

It’s equivalent and probably easier to read, esp as you can then just do:

case handler.handle_event(event, state) do

which is, at least IMHO, a lot easier to read than the pipeline syntax used there It contains lal the information needed in one line (e.g. what is the subject of the case) rather than having to track back…

I also noticed you have call of String.to_atom/1 on externally supplied data .. this is a great way to exhaust the atom table (you only get so many!), run out of entries, and DoS your app as an unhappy side-effect You should really look to replace that approach.

Finally .. as to where your binary “leaks” are coming from … I assume it is something like this:

• you get a block of json as a binary from the external service
• Jason parses that into smaller strings for your, but each of those smaller strings are just pointers into the bigger one .. and that prevents the GC from removing those binaries until the references to the smaller strings are dropped
• those smaller strings are being put into an ets table, so they stay forever until the entry is deleted from the ets table, which means the binaries stay forever until the entries are deleted in the ets table
• since coxir is updating those entries over time from many different json input binaries, each entry in the ets table(s) is keeping multiple json binaries in memory over their lifespan.

You can get around this by calling :binary.copy/1 on the specific sub-strings you are storing in the ets table to see if that is indeed the issue.

Honestly, not yet. I managed to lower the memory used by the entire module by reducing the amount of middles, but the main problem was, and still is, the memory / binary leak from GenServer.

The issue is that GenServer processes are consuming way too much memory, sadly.

Did you end up solving your problem? Quite an interesting thread.

Very interesting, will really have a look at it. I will later post here the results