Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory Leak #207

Closed
Joshua-F opened this issue Feb 1, 2016 · 4 comments
Closed

Memory Leak #207

Joshua-F opened this issue Feb 1, 2016 · 4 comments

Comments

@Joshua-F
Copy link

Joshua-F commented Feb 1, 2016

Running on Windows 10 64bit I noticed that when refreshing a game directory will cause the memory usage to continually raise and never go back down.

Initial memory usage on startup and going to a game directory.

After refreshing the directory 5 times.

After refreshing the directory 10 more times.

As you can see the memory usage goes up quite a bit from just refreshing the directory listing, which wouldn't be a problem if the memory usage would go back down after being garbage collected. I noticed this issue when I saw the program using over 1gb of memory one day. I haven't looked into this much myself so I'm not sure what the cause is.

@bastimeyer
Copy link
Member

Thanks for reporting...

I know that there are a couple of memory leaks. EmberData's data store for instance is hoarding records and is not removing them and there may also be some event listeners on DOM nodes which are not being removed properly. But this one comes a bit unexpected.

I've tried finding the source of this refresh memory leak, and I think I have found the issue, but I'm not entirely sure:
From what it looks like, there's a memory leak in Ember/HTMLBars when using each-loops inside if blocks.

This fix has been cherry picked into Ember 2.0.2, but I don't know if it is also included in the version which is currently being used (2.1.2). Github doesn't list the tags (see at the top), that's why I'm not sure.

Maybe a bump of the used Ember version can fix this issue. There were some other issues though when I tried out a newer Ember release last time.

I'm a little bit busy right now and don't have that much time in the next couple of weeks, so I don't know if I'm able to fix this any time soon. Let me see...

@bastimeyer
Copy link
Member

This is what I have tried so far fixing this: (will do more when I find the time)

  • Upgraded Ember and EmberData to 2.3.0
  • Moved the each-loop out of the if block
  • Used mutable component parameters
  • Replaced all used components of this route with simple non-component based handlebars code
  • Used simple list items without images or data bindings
  • Removed the image preloader from the route
  • Replaced the store.query call with a simple Promise.resolve() call that's returning a simple array of game objects

I had no success with any of these approaches. The used memory still keeps growing with each route refresh...

What surprised me the most is the last thing I tried with the resolved promise. Does this mean that there is a bug in Ember's Route.prototype.refresh() method? Or can this be related to the V8 version used by NWjs?

@bastimeyer
Copy link
Member

Ok, I've spent some more couple of hours investigating.

There was a leak in EmberData's RecordArrayManager causing all RecordArrays created when calling store.query to be held in cache. This is a known leak by the ED devs, but there's no fix for the upcoming versions yet, so I'm fixing this issue here by myself by using ED's private API (e270384)...

Sadly, the memory usage is still growing with each refresh. This seems to be an issue of NW.js 0.12.x, though. The GC doesn't seem to work properly, which has already been reported a dozen of times over at the NW.js repo.

I've tested the latest NW.js version 0.13.0-rc.1 and it seems to be working now. The GC properly cleans up after a few refreshes and the memory consumption is stable.

I will upgrade as soon as possible, but right now, it's not possible. The 0.13.0 release is not yet finished and the nwjs-builder also doesn't support the new version yet.

@Joshua-F
Copy link
Author

Joshua-F commented Mar 8, 2016

Thanks for looking into it, glad to see that it'll be fixed in a later version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants