Optimization of ActiveModel's match_attribute_method? #2075

Merged
merged 3 commits into from Jul 18, 2011

Conversation

Projects
None yet
3 participants
Contributor

lawrencepit commented Jul 15, 2011

In production mode we were measuring the number of objects created, because the GC was working too hard imo, esp. compared to rails2.

Typically we got these numbers per request, the top 5:

Total: 134949 samples
20460 15.2% 15.2% 22374 16.6% Object#match_attribute_method?
6907 5.1% 20.3% 38812 28.8% Array#map
5662 4.2% 24.5% 5662 4.2% Set#subtract
4359 3.2% 27.7% 5513 4.1% Set#merge
3762 2.8% 30.5% 18610 13.8% Object#read_attribute
3288 2.4% 32.9% 18607 13.8% ActiveRecord::Base#attributes

So for every request about 15% of all objects created, are created by the calls to match_attribute_method? in ActiveModel.

At 10 requests/second, this patch saves over 10 million objects per minute for us in production mode.

I'm however not that familiar with ActiveModel, and this is probably not the best way to solve this. But I thought I put this out there first and work from there. My main question atm is: are there times when this cache would need to be cleared?

Contributor

lawrencepit commented Jul 15, 2011

Update: we have a large test suite for our app, with this patch it finishes about 10% faster.

Contributor

josevalim commented Jul 15, 2011

Hey Lawrence! It looks good BUT I would move the method to the class
and cache on each class using @instance_class_variable. The current
implementation causes memory leaks in development (as each class is
different).

Thanks a lot!

Em quinta-feira, 14 de julho de 2011,
lawrencepitreply@reply.github.com
escreveu:

In production mode we were measuring the number of objects created, because the GC was working too hard imo, esp. compared to rails2.

Typically we got these numbers per request, the top 5:

 Total: 134949 samples
  20460  15.2%  15.2%    22374  16.6% Object#match_attribute_method?
   6907   5.1%  20.3%    38812  28.8% Array#map
   5662   4.2%  24.5%     5662   4.2% Set#subtract
   4359   3.2%  27.7%     5513   4.1% Set#merge
   3762   2.8%  30.5%    18610  13.8% Object#read_attribute
   3288   2.4%  32.9%    18607  13.8% ActiveRecord::Base#attributes

So for every request about 15% of all objects created, are created by the calls to match_attribute_method? in ActiveModel.

At 10 requests/second, this patch saves over 10 million objects per minute for us in production mode.

I'm however not that familiar with ActiveModel, and this is probably not the best way to solve this. But I thought I put this out there first and work from there. My main question atm is: are there times when this cache would need to be cleared?

Reply to this email directly or view it on GitHub:
#2075

José Valim
www.plataformatec.com.br
Founder and Lead Developer
*

Contributor

lawrencepit commented Jul 16, 2011

I see, modified the code in commit c3dd4c6.

Contributor

josevalim commented Jul 16, 2011

class_attributes are shared and inherited, which means everyone would share the cache you created. I would simply remove it and use:

private

def attribute_method_matchers_cache
  @attribute_method_matchers_cache ||= {}
end

Adding some docs why the cache is important would be a +1 as well. If you prefer, you can go ahead and do these changes as well, otherwise I will review and merge it when I am back from vacation. In any case, thanks a lot!

Contributor

lawrencepit commented Jul 17, 2011

Moved the method to private and added some doc.

josevalim added a commit that referenced this pull request Jul 18, 2011

Merge pull request #2075 from lawrencepit/match_attribute_method
Optimization of ActiveModel's match_attribute_method?

@josevalim josevalim merged commit 44e83ac into rails:master Jul 18, 2011

kir commented Nov 4, 2011

Can we get this incorporated into 3-1-stable branch?

Thanks,
KIR

kir added a commit to kir/rails that referenced this pull request Nov 4, 2011

eac pushed a commit to grosser/rails that referenced this pull request Jun 5, 2013

Issue #2075 Optimization of ActiveModel's match_attribute_method?
Conflicts:

	activemodel/lib/active_model/attribute_methods.rb

eac pushed a commit to grosser/rails that referenced this pull request Jun 6, 2013

Issue #2075 Optimization of ActiveModel's match_attribute_method?
Conflicts:

	activemodel/lib/active_model/attribute_methods.rb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment