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

rm "at_exit" #28

Closed
wants to merge 1 commit into from
Closed

rm "at_exit" #28

wants to merge 1 commit into from

Conversation

ruseel
Copy link

@ruseel ruseel commented Jul 15, 2015

at_exit seems to leak memory.

checked with below script
(copied from SamSaffron's
rubyjs/therubyracer#336)

require 'objspace'
require 'memory_profiler'
require 'fluent-logger'

def rss
 `ps -eo pid,rss | grep #{Process.pid} | awk '{print $2}'`.to_i
end

MemoryProfiler::Helpers.full_gc
puts "rss: #{rss} live objects #{GC.stat[:heap_live_slots]}"

20.times do
  500.times { |i|
    Fluent::Logger::FluentLogger.new.post("tag", {"a"=>1})
  }

  MemoryProfiler::Helpers.full_gc
  puts "rss: #{rss} live objects #{GC.stat[:heap_live_slots]}"
end

before this patch

rss: 19528 live objects 38181
rss: 20836 live objects 46999
rss: 21108 live objects 54998
rss: 21948 live objects 62998
rss: 22992 live objects 70998
rss: 24192 live objects 78998
rss: 25248 live objects 86998
rss: 26324 live objects 94998
rss: 27432 live objects 102998
rss: 28556 live objects 110998
...
...

after

rss: 19840 live objects 38181
rss: 20988 live objects 38998
rss: 21048 live objects 38997
rss: 21056 live objects 38997
rss: 21116 live objects 38997
rss: 21116 live objects 38997
rss: 21116 live objects 38997
rss: 21116 live objects 38997
rss: 21116 live objects 38997
rss: 21116 live objects 38997
rss: 21116 live objects 38997
rss: 21116 live objects 38997
...
...

at_exit seems to leak memory.

checked with below script
(copied from SamSaffron's
rubyjs/therubyracer#336)

```
require 'objspace'
require 'memory_profiler'
require 'fluent-logger'

def rss
 `ps -eo pid,rss | grep #{Process.pid} | awk '{print $2}'`.to_i
end

MemoryProfiler::Helpers.full_gc
puts "rss: #{rss} live objects #{GC.stat[:heap_live_slots]}"

20.times do
  500.times { |i|
    Fluent::Logger::FluentLogger.new.post("tag", {"a"=>1})
  }

  MemoryProfiler::Helpers.full_gc
  puts "rss: #{rss} live objects #{GC.stat[:heap_live_slots]}"
end
```

before this patch

```
rss: 19528 live objects 38181
rss: 20836 live objects 46999
rss: 21108 live objects 54998
rss: 21948 live objects 62998
rss: 22992 live objects 70998
rss: 24192 live objects 78998
rss: 25248 live objects 86998
rss: 26324 live objects 94998
rss: 27432 live objects 102998
rss: 28556 live objects 110998
...
...

```

after

```
rss: 19840 live objects 38181
rss: 20988 live objects 38998
rss: 21048 live objects 38997
rss: 21056 live objects 38997
rss: 21116 live objects 38997
rss: 21116 live objects 38997
rss: 21116 live objects 38997
rss: 21116 live objects 38997
rss: 21116 live objects 38997
rss: 21116 live objects 38997
rss: 21116 live objects 38997
rss: 21116 live objects 38997
...
...
```
@repeatedly
Copy link
Member

It seems at_exit's slot isn't GCed in one process.
Does this become a problem?
From our experience, user doesn't create lots of FluentLogger instance in one process.

Anyway, if we merge this PR, we should announce the users because this change breaks existing FluentLogger behaviour.

@ruseel
Copy link
Author

ruseel commented Aug 17, 2016

Does this become a problem?
From our experience, user doesn't create lots of FluentLogger instance in one process.

I did create a lots of FluentLogger instance. and now I see that is definitely not good as you said.

@ruseel ruseel closed this Aug 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants