search

monome / norns

norns is many sound instruments.
http://monome.org
GNU General Public License v3.0
630 stars 145 forks source link

matron suspected memory leak #747

Closed ranch-verdin closed 5 years ago

ranch-verdin commented 5 years ago

I have been running the following stress test in order to ensure the stability of "./engine", the host for sgynth:

stress test: https://gist.github.com/ranch-verdin/7c641533185167eee0d14ca264c3692a sgynth: https://github.com/ranch-verdin/sgynth

Under continuous stress testing matron bombs out after roughly 1 1/2 hours. I observe steadily increasing matron memory consumption reported by top.

Unclear at this stage whether this is really a memory leak, or whether lua iterative gc can't keep up with the plainly ridiculous load and prefers to increase memory usage rather than violate timing constraints (I would call this a feature, not a bug!).

Suggest a modified test to alternate 1s extreme stress burst with 1s quiet to give lua gc a fighting chance, otherwise looks like a job for valgrind.

catfact commented 5 years ago

i would look at the event queue. if main loop can't process events as fast as they are added, memory usage will grow without bound...

ranch-verdin commented 5 years ago

during the test, cpu usage of matron is only 10% and overall usage of core 1 is 20%. I assume from this figure that main loop would be expected to keep up with the (admittedly large) number of events I'm flooding into the queue?

ranch-verdin commented 5 years ago

reduced frequency of events in test from 'microsound' to something approaching 'extreme sequencing'. Just kicked off an all-day soak, appears that matron memory use already expanding, only more slowly.

ranch-verdin commented 5 years ago

hmm... could this be the culprit? https://github.com/monome/norns/blob/master/matron/src/oracle.c#L270

ngwese commented 5 years ago

That does seem suspect. I’m wondering if a run under valgrind might be in order to catch any other cases.

catfact commented 5 years ago

hn, I thought I remembered cleaning this up once already...

...yep, but I guess I missed that one? https://github.com/monome/norns/commit/0ad51581d530e86b15cfea6072ea484f2435ea39

memcheck is definitely in order

catfact commented 5 years ago

Ok yea, we’ve got a classic bungle here with a function that frees its argument (o_send) but which is inconsistently used. Let’s remove it, see what shakes out, and never do that again

ranch-verdin commented 5 years ago

short valgrind run after a patch to catch the obvious one at oracle.c#L270

https://gist.github.com/ranch-verdin/a403453ee4d8ef177e79eabe6798f6e7

I'm a valgrind noob, so don't really know what I'm looking at - going to have a look later on.

catfact commented 5 years ago

obvious memleaks patched in oracle, events

closing this for now