Memory leak with new typhoeus version #20

Closed
rb2k opened this Issue Jul 18, 2012 · 17 comments

Projects

None yet

4 participants

@rb2k
rb2k commented Jul 18, 2012

I did a bundle update on an app of mine and noticed that the app started leaking memory pretty badly.

While page_rankr didn't change, I noticed typhoeus change.

No mem leak: typhoeus (0.3.3)
Leaking memory: typhoeus (0.4.2)

Anybody else notice something similar?
I use page-ranks (the whole lot) and incoming links (yahoo) for a lot of changing domains

I'll try to come up with a minimal testcase.
I'm not sure if it's the way that typhoeus is used or typhoeus itself.

update: I also see that from 0.3.3 to 0.4.2 they added FFI as a dependency. I recall that typhoeus wanted to switch the c extension system, I assume that could easily leak some memory in there. Maybe @i0rek or @dbalatero from @typhoeus know something about that? :)

@blatyo
Owner
blatyo commented Aug 3, 2012

Did you resolve this? If not, perhaps you could try running perftools against your code to see what is causing the leak. Profiling for objects rather than walltime is probably what you want.

@i0rek
i0rek commented Aug 3, 2012

Hi @rb2k,
I'm not aware of a memory leak. Will look into it as soon I've something to reproduce.

@rb2k
rb2k commented Aug 3, 2012

I did resolve this by just locking down typhoeus to the 0.3.3 version.
I haven't been able to create a minimal example of this behavior.

@i0rek
i0rek commented Aug 10, 2012

thats a pity.

@blatyo
Owner
blatyo commented Oct 21, 2012

Closing this, since the discussion had died out, the issue was most likely related to typheous, and there are new versions of typheous now.

@blatyo blatyo closed this Oct 21, 2012
@thoughtpunch

Worth noting at least one other person has observed this as well. Since our app has typhoeus version locked at 0.4.2 because of dependency conflicts we have had to stop using PageRankr in production. Hopefully this goes away completely with later versions.

@i0rek
i0rek commented Nov 7, 2012

@thoughtpunch I totally forgot about this issue, but the problem came up again and I can now reproduce it. I'm working on that right now!

@blatyo
Owner
blatyo commented Nov 7, 2012

@i0rek, is this confirmed as an issue with Typheous or were you able to reproduce with PageRankr?

@i0rek
i0rek commented Nov 8, 2012

@blatyo It is a Typhoeus issue. There are two problems:

  1. Typhoeus::Hydra uses too much memory
  2. Typhoeus leaks memory.

Typhoeus 0.5 changes some internals so that single requests aren't using the hydra any more. For single requests Typhoeus still leaks memory, but only very little. From what I've seen in your code you could upgrade to 0.5 and use single requests instead of the hydra here: https://github.com/blatyo/page_rankr/blob/master/lib/page_rankr/tracker.rb#L52.

def run
  request.run

  tracked
end

These changes have no effects on <0.5 because its using the hydra there, even for single requests.

@blatyo blatyo reopened this Nov 8, 2012
@blatyo
Owner
blatyo commented Nov 8, 2012

@i0rek Thanks for the info and I can certainly make that change.

I have couple questions for clarification:

  • You say for a single request Typhoeus leaksa little memory; about how much is that?
  • For multiple requests through hydra, is it leaking the amount for each single request and some large amount for hyrda?
  • What is the size of that leak if any?

Thanks

@i0rek
i0rek commented Nov 8, 2012

I'm still investigating, but I suggest assuming the worst case. It leaks approximately 3MB per hydra plus 8kb per request. If your using the hydra you have to add the numbers.

@i0rek
i0rek commented Nov 9, 2012

See my comment here: typhoeus/typhoeus#229 (comment).

TLDR; I was wrong, I did not found a memory leak.

@blatyo
Owner
blatyo commented Nov 9, 2012

Awesome!

@blatyo
Owner
blatyo commented Dec 2, 2012

@rb2k @thoughtpunch I'm going to be changing the internal implementation to no longer use typheous. This should resolve any issues you have.

@rb2k
rb2k commented Dec 2, 2012

Which HTTP library are you planning on switching to? I've had good experiences (performance wise) with httpclient. Plus it doesn't need C extensions.

@blatyo
Owner
blatyo commented Dec 2, 2012

Undecided, but it will not require a C extension. Whatever I choose needs to have good support for proxies.

@blatyo
Owner
blatyo commented Dec 18, 2012

Version 4.0.0 should resolve these issues. If not, please open a new issue.

Thanks

@blatyo blatyo closed this Dec 18, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment