search

wilk / microjob

A tiny wrapper for turning Node.js worker threads into easy-to-use routines for heavy CPU loads.
https://wilk.github.io/microjob/
MIT License
2.02k stars 47 forks source link

High core count CPU causes misleading memory leak warning #36

Closed ghost closed 5 years ago

ghost commented 5 years ago

When testing this library out I received the warning MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 17 error listeners added. Use emitter.setMaxListeners() to increase limit

I puzzled over why my code was adding so many listeners, before realising it was caused by microjob. I was about to dismiss microjob and look for an alternative when I realised that the default numbers of workers is equal to the number of OS threads. My machine has 16, which explains this behaviour.

I see that I can use MAX_WORKERS to set the number lower, but others may not realise this, or even realise that their core count is what's causing the issue.

Perhaps the number of listeners can be increased on >10 core CPUs (as node will warn after 10 listeners), or a custom runtime warning could be included explaining the cause.

Additional request, could we perhaps have a way to set max workers via an argument rather than requiring an environment variable? This would allow me to include it in an application configuration file instead of needing to modify my launch command.

wilk commented 5 years ago

Hi @benji7425 ! Thank you for exposing this issue! Yes, I think both of your solutions are valid. Also yes, I'm going to move the environment variable inside the worker pool configurations.

Stay tuned 💪

wilk commented 5 years ago

Implemented both on version v0.5.0 🎉