Out of control memory usage

[Enverex]

I've set this up on two machines now, a desktop x86_64 system and an ARM development board and in both cases, within a few minutes, it had used up all of the RAM on the system.

Here it is on the ARM system (with 2GB of RAM) after roughly 3 minutes:

PID USER PR NI VIRT RES %CPU %MEM TIME+ S COMMAND 5844 ben 20 0 1815.3m 1.581g 0.0 81.2 2:37.55 S python2 /home/ben/build/mycroft-core/mycroft/skills/main.py

Within a few seconds of pasting that, the system killed the process due to OOM.

[penrods]

We've looked in to this some. I suspect two things:

1) The skill service monitors for changes to each skill's folder and reloads them. It was considering the creation of .pyc (compiled Python bytecode files) a 'change'. So at the bare minimum the first time you ran any skill it would execute the init.py and generate a init.pyc, causing the skill to unload and reload at least once. This has been addressed since the last release in PR #1002, already merged into 'dev' and ready for the next release.

2) The fallback-aiml skill has been added as a 'default' skill. Upon first loading the AIML engine does a bunch of parsing of the .aiml files inside that skill. This could lead to a few things

• The skill restart(s) of 1 would interrupt this compile process and might leave dangling references in memory, bloating the mycroft-skills process
• Even when things work cleanly, the AIML skill does take up a lot more memory than the previous default skills.

We are still considering whether AIML should remain a default. you can disable it by renaming the file: /opt/mycroft/skills/fallback-aiml/__init__.py to: /opt/mycroft/skills/fallback-aiml/__init__.py.disabled

[JarbasAI]

perhaps the aiml skill should provide the saved brain file instead of making it in the first run

we could make a minimal brain, the current AIML skill uses a lot of files that could be curated to at least remove things that would be triggered by default skills (jokes come to mind) instead of using direct copy pasta of ALICE/mitsuku chatbot

perhaps a repository for extra AIML files could be made and keep the default skill minimal

a self.reload_skill = False would also help in this case with not reloading

[forslund]

I've disabled writing/loading brain file for the moment and I'm forcing unload of aiml-kernel at shutdown as a quick fix for this specific problem. Skill should be updated automatically. A forced upgrade can be made using ./msm/msm upgrade. Now it's using the aiml-files only which is a bit slower starting up than using the brain file as @JarbasAI suggests.

I still need to find the root cause why the loaded data isn't cleaned up when shutting down skills, if it's a bug in the aiml module or if there remains references to the old skill object after reload.

[forslund]

Short update, I've checked and the unloaded skills doesn't seem to get garbage collected (added a __del__() with a debug print and enforced garbage collection. The refcount of the skill objects after shutdown:

References to AudioRecordSkill remaining: 11
References to Playback Control Skill remaining: 9
References to RssSkill remaining: 7
References to DesktopLauncherSkill remaining: 1
References to SkillInstallerSkill remaining: 3
References to PairingSkill remaining: 4
References to WikipediaSkill remaining: 3

Some of these can be caused by self references but likely not all.

[forslund]

Created a minimal test for the issue: https://gist.github.com/forslund/44eccf583091afacb810e7cae08cafad

[forslund]

One obvious thing is that the handler for 'stop.mycroft' isn't teared down but even with this I don't see any difference.

Edit: I can remove it no problem. problem is only with methods registered with add_event()

[forslund]

http://imgur.com/a/0TZ71 Shows the problem I think. In this case I've used the self.add_event() to add the 'mycroft.stop' message so it would be removed from the event emitter together with normal methods during shutdown. However the wrapper function seems to be hanging around despite it being removed from the event emitter.

If I skip the .add_event() and use a direct self.emitter.on() to set the handler the handler can be removed.

[forslund]

I'm closing this since #1014 resolved a number of reference leaks, and #1002 resolved an issue which caused skills to reload multiple times, (making the reference leaks multiple times worse)

Feel free to reopen if you feel it's still an issue.