maxdemarzi / neography

A thin Ruby wrapper to the Neo4j Rest API
MIT License
602 stars 135 forks source link

I think we may have a memory leak #159

Open maxdemarzi opened 10 years ago

maxdemarzi commented 10 years ago

Can we double check https://github.com/maxdemarzi/neography/blob/master/lib/neography/rest/node_paths.rb for a potential leak?

I'm getting reports of R14s on Heroku and the issue may be lots of "paths" being generated.

maxdemarzi commented 10 years ago

See http://pastebin.com/2hkJ8LcJ

/Users/kwent/.rvm/gems/ruby-2.1.0/bundler/gems/neography-a4931670b7ff/lib/neography/rest/paths.rb String 16170

rdvdijk commented 10 years ago

Interesting. The two lines that stand out are:

/Users/kwent/.rvm/gems/ruby-2.1.0/bundler/gems/neography-a4931670b7ff/lib/neography/connection.rb          String                                                                                     17742

(17742 String objects in connection.rb)

and

/Users/kwent/.rvm/gems/ruby-2.1.0/bundler/gems/neography-a4931670b7ff/lib/neography/rest/paths.rb          String                                                                                     16170

(16170 String objects in paths.rb)

Any other concerns?

How can we recreate a similar log file with object counts? That 's useful during for debugging.

rdvdijk commented 10 years ago

I guess this StackOverflow issue is related?

kwent commented 10 years ago

Yeah i opened this issue few days ago and was hopping to receive help without success :( .

maxdemarzi commented 10 years ago

What if we refactor all those classes into modules?

That's my current plan I think...

rdvdijk commented 10 years ago

@maxdemarzi What would that fix? I think we should first identify what the exact problem is. What are all those String objects?

mperham commented 10 years ago

I presume you've already seen this. https://github.com/mperham/sidekiq/issues/1421#issuecomment-33746157

kwent commented 10 years ago

Yes. But even running on Ruby 2.0.0, we noticed memory leak.

maxdemarzi commented 10 years ago

Latest dump => http://pastebin.com/VBPaV5mD

So it looks like it may not be the Rest class at all, but something borked with the "Phase 2" functionality.

steveodom commented 10 years ago

Any update on this?