Hi, I''ve uploaded a patch to optimize eager loading, which changes the code to spend less time in ruby - http://dev.rubyonrails.org/ticket/10011 The problem is because of following line in associations.rb:#construct_associations : collection.target.push(association) unless collection.target.include?(association) This basically compares every processed object with the new object, to see if it hasn''t been instansiated already. For testing, I created two simple models : class Person < ActiveRecord::Base has_many :items end class Item < ActiveRecord::Base belongs_to :person end Data : 1 Person and 1000 items belonging to that person. Performance Script : http://pastie.caboo.se/111774 Before the patch : http://m.onkey.org/before.html After the patch : http://m.onkey.org/after.html Please test the patch if you can and also post your comments/suggestions. -- Cheers! - Pratik http://m.onkey.org --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
Looks interesting. I don''t have time to test this but have you tried to eager load several has_many associations at once (so there are M*N result rows) and compare the memory consumption of the new approach with the old one? On P, 2007-10-28 at 18:34 +0000, Pratik wrote:> Hi, > > I''ve uploaded a patch to optimize eager loading, which changes the > code to spend less time in ruby - > http://dev.rubyonrails.org/ticket/10011 > > The problem is because of following line in > associations.rb:#construct_associations : > collection.target.push(association) unless > collection.target.include?(association) > > This basically compares every processed object with the new object, to > see if it hasn''t been instansiated already. > > For testing, I created two simple models : > > class Person < ActiveRecord::Base > has_many :items > end > > class Item < ActiveRecord::Base > belongs_to :person > end > > Data : 1 Person and 1000 items belonging to that person. > Performance Script : http://pastie.caboo.se/111774 > Before the patch : http://m.onkey.org/before.html > After the patch : http://m.onkey.org/after.html > > Please test the patch if you can and also post your comments/suggestions.--~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
Pratik wrote:> I''ve uploaded a patch to optimize eager loading, which changes the > code to spend less time in ruby - > http://dev.rubyonrails.org/ticket/10011 > > The problem is because of following line in > associations.rb:#construct_associations : > collection.target.push(association) unless > collection.target.include?(association) > > This basically compares every processed object with the new object, to > see if it hasn''t been instansiated already.How does this compare to Fred Cheung''s patch? http://groups.google.com/group/rubyonrails-core/msg/b4da8cedcd73eaae -- We develop, watch us RoR, in numbers too big to ignore. --~--~---------~--~----~------------~-------~--~----~ 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 29 Oct 2007, at 01:39, Mark Reginald James wrote:> > Pratik wrote: > >> I''ve uploaded a patch to optimize eager loading, which changes the >> code to spend less time in ruby - >> http://dev.rubyonrails.org/ticket/10011 >> >> The problem is because of following line in >> associations.rb:#construct_associations : >> collection.target.push(association) unless >> collection.target.include?(association) >> >> This basically compares every processed object with the new >> object, to >> see if it hasn''t been instansiated already. > > How does this compare to Fred Cheung''s patch? > > http://groups.google.com/group/rubyonrails-core/msg/b4da8cedcd73eaae >That was rather fugly :-) Array#uniq! basically builds up a hash and then collects the things in the hash, so it should be pretty similar. The one possible area of concern (as pointed out by tarmo) is memory consumption with a large result set. Fredd> -- > We develop, watch us RoR, in numbers too big to ignore. > > > >--~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---