{"action":"created","issue":{"url":"https://api.github.com/repos/Bertram25/ValyriaTear/issues/480","labels_url":"https://api.github.com/repos/Bertram25/ValyriaTear/issues/480/labels{/name}","comments_url":"https://api.github.com/repos/Bertram25/ValyriaTear/issues/480/comments","events_url":"https://api.github.com/repos/Bertram25/ValyriaTear/issues/480/events","html_url":"https://github.com/Bertram25/ValyriaTear/issues/480","id":97017948,"number":480,"title":"Luabind + Memleak fix discussion","user":{"login":"Bertram25","id":765917,"avatar_url":"https://avatars.githubusercontent.com/u/765917?v=3","gravatar_id":"","url":"https://api.github.com/users/Bertram25","html_url":"https://github.com/Bertram25","followers_url":"https://api.github.com/users/Bertram25/followers","following_url":"https://api.github.com/users/Bertram25/following{/other_user}","gists_url":"https://api.github.com/users/Bertram25/gists{/gist_id}","starred_url":"https://api.github.com/users/Bertram25/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Bertram25/subscriptions","organizations_url":"https://api.github.com/users/Bertram25/orgs","repos_url":"https://api.github.com/users/Bertram25/repos","events_url":"https://api.github.com/users/Bertram25/events{/privacy}","received_events_url":"https://api.github.com/users/Bertram25/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/Bertram25/ValyriaTear/labels/Comment%20or%20Feedback","name":"Comment or Feedback","color":"DDDDDD"},{"url":"https://api.github.com/repos/Bertram25/ValyriaTear/labels/Refactoring","name":"Refactoring","color":"fbca04"}],"state":"open","locked":false,"assignee":{"login":"Bertram25","id":765917,"avatar_url":"https://avatars.githubusercontent.com/u/765917?v=3","gravatar_id":"","url":"https://api.github.com/users/Bertram25","html_url":"https://github.com/Bertram25","followers_url":"https://api.github.com/users/Bertram25/followers","following_url":"https://api.github.com/users/Bertram25/following{/other_user}","gists_url":"https://api.github.com/users/Bertram25/gists{/gist_id}","starred_url":"https://api.github.com/users/Bertram25/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Bertram25/subscriptions","organizations_url":"https://api.github.com/users/Bertram25/orgs","repos_url":"https://api.github.com/users/Bertram25/repos","events_url":"https://api.github.com/users/Bertram25/events{/privacy}","received_events_url":"https://api.github.com/users/Bertram25/received_events","type":"User","site_admin":false},"milestone":{"url":"https://api.github.com/repos/Bertram25/ValyriaTear/milestones/8","html_url":"https://github.com/Bertram25/ValyriaTear/milestones/1.1%20Release","labels_url":"https://api.github.com/repos/Bertram25/ValyriaTear/milestones/8/labels","id":817964,"number":8,"title":"1.1 Release","description":"This release is a small step forward release including mainly the addition of a skill tree, instead of fixed skill obtained on level up, and the rebalancing of the game to be more player-friendly.\r\n\r\nAlso, some small to medium refactoring will have to take place before I move further with content.","creator":{"login":"Bertram25","id":765917,"avatar_url":"https://avatars.githubusercontent.com/u/765917?v=3","gravatar_id":"","url":"https://api.github.com/users/Bertram25","html_url":"https://github.com/Bertram25","followers_url":"https://api.github.com/users/Bertram25/followers","following_url":"https://api.github.com/users/Bertram25/following{/other_user}","gists_url":"https://api.github.com/users/Bertram25/gists{/gist_id}","starred_url":"https://api.github.com/users/Bertram25/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Bertram25/subscriptions","organizations_url":"https://api.github.com/users/Bertram25/orgs","repos_url":"https://api.github.com/users/Bertram25/repos","events_url":"https://api.github.com/users/Bertram25/events{/privacy}","received_events_url":"https://api.github.com/users/Bertram25/received_events","type":"User","site_admin":false},"open_issues":16,"closed_issues":62,"state":"open","created_at":"2014-10-08T06:59:14Z","updated_at":"2015-07-24T10:02:20Z","due_on":null,"closed_at":null},"comments":4,"created_at":"2015-07-24T10:02:20Z","updated_at":"2015-07-26T00:38:46Z","closed_at":null,"body":"@akien-mga @authenticate @endoalir \r\n\r\nI'm opening the following issue because of #479, #463, and also #354 most notably, which are all the symptoms of a recurrent problem around Luabind, garbage collection, and overall memory handling.\r\n\r\nI'll thus propose a suite of tasks aiming at improving the current situation and/or give the tools to understand and fix it.\r\n\r\nThis issue is also there to get your feed-back, which has been invaluable so far to me, in case I'm missing something or if you have any own idea on all this.\r\n\r\n1. *MemLeak and Lua:*\r\nAs it seems lua and the memleak are linked, I'll first work on adding a debug permitting to dump the lua stack and finally be able to know what is staying in memory and why. This, I guess, should help to decide what's next.\r\n\r\n2. Move the battle media resource in the global ones. The Global media resources are loaded once, but the battle media are loaded at each battles. This doesn't make sense. One sole global loader makes much more sense.\r\n\r\n3. *Luabind:*\r\nDepending on whether luabind is the actual culprit about the memleak or not, it'll have to stay if possible.\r\nIf it's the case, and arf, as this one is becoming some kind of nasty stuff and as @akien-mga is pointing out, patching one's own version of luabind, especially intricated within the rest of the code is considered a bad practice, even if a very common one.\r\nWhile I consider luabind to be upstream-dead for obvious reason, and as VT is heavily relying on it for many things so far, it'll have to stay for now. But let's make the current situation clearer about this one:\r\nI propose to create a luabind repository and move upstream luabind 0.9 branch there.\r\nThen, I'll readd clean commits about code cleanups, lua 5.2 support, latest boost support , and c++11 support.\r\nAt the end, I'll also add the function call in the constructor PR as noticed by @endoalir but from https://github.com/luabind/luabind/pull/26 as it is indeed more complete.\r\nIf you see some other improvements looking relevant as for luabind, I'll have a look at them with pleasure. :)\r\nTo answer @akien-mga point of view on repository code duplication, unfortunately, as I don't think something will happen on luabind state soon, I have to become the upstream somehow, but I'll then indeed need help to gather useful improvements made since latest upstream version.\r\nTo answer another question that will raise soon, using someone else's more active fork may also be a solution, such as @Oberon00's or @rpavlik's BUT, and there is a big but, I have to make sure of the dependency stability and thus will have to manage what I import form them, which in the end is the same as maintaining one's own fork.\r\n\r\n4. small refactorings to import:\r\nThere are interesting bits of refactoring in ImageMemory and ustring from @bmorel that I'll have to cleanup and import:\r\nhttps://github.com/bmorel/ValyriaTear/commits/refactoring\r\n\r\n5. Other things such as using shared_ptr when necessary, ...\r\n\r\nWDYT?"},"comment":{"url":"https://api.github.com/repos/Bertram25/ValyriaTear/issues/comments/124922089","html_url":"https://github.com/Bertram25/ValyriaTear/issues/480#issuecomment-124922089","issue_url":"https://api.github.com/repos/Bertram25/ValyriaTear/issues/480","id":124922089,"user":{"login":"endoalir","id":1078142,"avatar_url":"https://avatars.githubusercontent.com/u/1078142?v=3","gravatar_id":"","url":"https://api.github.com/users/endoalir","html_url":"https://github.com/endoalir","followers_url":"https://api.github.com/users/endoalir/followers","following_url":"https://api.github.com/users/endoalir/following{/other_user}","gists_url":"https://api.github.com/users/endoalir/gists{/gist_id}","starred_url":"https://api.github.com/users/endoalir/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/endoalir/subscriptions","organizations_url":"https://api.github.com/users/endoalir/orgs","repos_url":"https://api.github.com/users/endoalir/repos","events_url":"https://api.github.com/users/endoalir/events{/privacy}","received_events_url":"https://api.github.com/users/endoalir/received_events","type":"User","site_admin":false},"created_at":"2015-07-26T00:38:46Z","updated_at":"2015-07-26T00:38:46Z","body":"I've been investigating this today. This morning, I played through the whole story, tracking memory usage every 5 minutes. By the time I got past the end credits and back to the title screen, memory usage was about 650 MB. \r\n\r\nAfterwards, I ran it under valgrind massif and got as far as I could. Unfortunately the game froze in the middle of a cut scene with Banesore. Massif makes the program run sluggish, and apparently the poor performance seems to affect some timing triggers. During this run, heap usage peaked around 283 MB. Of that: 49.20% was allocated by Lua, resulting from calls to lua_pcall in script_read.cpp line 71. Additionally 33.71% of it results from calls to alBufferData in audio_descriptor.h line 100. All other heap objects allocated in the program source make up 9.78%, just tiny things.\r\n\r\nWhen I searched specifically for memory that was leaked, memory unreferenced anywhere, I wasn't able to find much. Valgrind itself crashed on me during one leak hunt session. I did find shop.cpp leaks a small but not insignificant amount of memory but other than that not much from investigating that route."}}