Hi All, I tracked down a memory leak (600~1000kb per request for our setup) originated from this commit, commit ca6d71753f3a2e8a0a29108b7c55ba3b7c8cd943 Date: Fri Aug 22 12:19:29 2008 -0500 Deprecate allow_concurrency and make it have no effect It''s a commit that deprecated `''allow_concurrency'', but looks like it kept all them mechanism intact (which is kinda weird). The patch below stops the leaking, but I don''t really know why Thread.current is still in there, or why it''s leaking. It surprised me that it''s from so far back. Howard diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/ active_record/base.rb index a36a137..df31a3d 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -2023,7 +2023,8 @@ module ActiveRecord #:nodoc: end def scoped_methods #:nodoc: - scoped_methods = (Thread.current[:scoped_methods] ||= {}) + @scoped_methods ||= {} + scoped_methods = @scoped_methods scoped_methods[self] ||= [] end --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
We still need to make sure it is threadsafe. Can you run please your benchmarks against http://gist.github.com/22713 On Nov 5, 8:50 pm, Howard Yeh <hay...@gmail.com> wrote:> Hi All, > > I tracked down a memory leak (600~1000kb per request for our setup) > originated from this commit, > > commit ca6d71753f3a2e8a0a29108b7c55ba3b7c8cd943 > Date: Fri Aug 22 12:19:29 2008 -0500 > > Deprecate allow_concurrency and make it have no effect > > It''s a commit that deprecated `''allow_concurrency'', but looks like it > kept all them mechanism intact (which is kinda weird). The patch below > stops the leaking, but I don''t really know why Thread.current is still > in there, or why it''s leaking. > > It surprised me that it''s from so far back. > > Howard > > diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/ > active_record/base.rb > index a36a137..df31a3d 100755 > --- a/activerecord/lib/active_record/base.rb > +++ b/activerecord/lib/active_record/base.rb > @@ -2023,7 +2023,8 @@ module ActiveRecord #:nodoc: > end > > def scoped_methods #:nodoc: > - scoped_methods = (Thread.current[:scoped_methods] ||= {}) > + @scoped_methods ||= {} > + scoped_methods = @scoped_methods > scoped_methods[self] ||= [] > end--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
On Nov 6, 1:41 pm, Joshua Peek <j...@joshpeek.com> wrote:> We still need to make sure it is threadsafe. Can you run please your > benchmarks againsthttp://gist.github.com/22713cool, that stopped the leaking. I still don''t see how it''s leaking though. Would you elaborate?> > On Nov 5, 8:50 pm, Howard Yeh <hay...@gmail.com> wrote: > > > Hi All, > > > I tracked down a memory leak (600~1000kb per request for our setup) > > originated from this commit, > > > commit ca6d71753f3a2e8a0a29108b7c55ba3b7c8cd943 > > Date: Fri Aug 22 12:19:29 2008 -0500 > > > Deprecate allow_concurrency and make it have no effect > > > It''s a commit that deprecated `''allow_concurrency'', but looks like it > > kept all them mechanism intact (which is kinda weird). The patch below > > stops the leaking, but I don''t really know why Thread.current is still > > in there, or why it''s leaking. > > > It surprised me that it''s from so far back. > > > Howard > > > diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/ > > active_record/base.rb > > index a36a137..df31a3d 100755 > > --- a/activerecord/lib/active_record/base.rb > > +++ b/activerecord/lib/active_record/base.rb > > @@ -2023,7 +2023,8 @@ module ActiveRecord #:nodoc: > > end > > > def scoped_methods #:nodoc: > > - scoped_methods = (Thread.current[:scoped_methods] ||= {}) > > + @scoped_methods ||= {} > > + scoped_methods = @scoped_methods > > scoped_methods[self] ||= [] > > end--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
Apparently, the GC does always properly cleanup the values for thread locals. I''d consider a bug in the GC. On Nov 6, 4:52 pm, Howard Yeh <hay...@gmail.com> wrote:> On Nov 6, 1:41 pm, Joshua Peek <j...@joshpeek.com> wrote: > > > We still need to make sure it is threadsafe. Can you run please your > > benchmarks againsthttp://gist.github.com/22713 > > cool, that stopped the leaking. > > I still don''t see how it''s leaking though. Would you elaborate? > > > > > On Nov 5, 8:50 pm, Howard Yeh <hay...@gmail.com> wrote: > > > > Hi All, > > > > I tracked down a memory leak (600~1000kb per request for our setup) > > > originated from this commit, > > > > commit ca6d71753f3a2e8a0a29108b7c55ba3b7c8cd943 > > > Date: Fri Aug 22 12:19:29 2008 -0500 > > > > Deprecate allow_concurrency and make it have no effect > > > > It''s a commit that deprecated `''allow_concurrency'', but looks like it > > > kept all them mechanism intact (which is kinda weird). The patch below > > > stops the leaking, but I don''t really know why Thread.current is still > > > in there, or why it''s leaking. > > > > It surprised me that it''s from so far back. > > > > Howard > > > > diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/ > > > active_record/base.rb > > > index a36a137..df31a3d 100755 > > > --- a/activerecord/lib/active_record/base.rb > > > +++ b/activerecord/lib/active_record/base.rb > > > @@ -2023,7 +2023,8 @@ module ActiveRecord #:nodoc: > > > end > > > > def scoped_methods #:nodoc: > > > - scoped_methods = (Thread.current[:scoped_methods] ||= {}) > > > + @scoped_methods ||= {} > > > + scoped_methods = @scoped_methods > > > scoped_methods[self] ||= [] > > > end--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---