Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #11109 (closed enhancement: fixed)

Opened 5 months ago

Last modified 4 months ago

[PATCH] Improve associations performance by avoiding named block arguments

Reported by: adymo Assigned to: technoweenie
Priority: normal Milestone: 2.x
Component: ActiveRecord Version: edge
Severity: normal Keywords:
Cc:

Description

In Ruby it's more memory efficient (and more faster) to avoid passing named block arguments. Depending on the block context the binding object required to create a named block may be quite large. And in AR models with associations it is indeed large.

This patch avoids named blocks in associations by replacing all occurences of: 

    def foo(&block)
        bar(&block)
    end

with 

    def foo
        bar { |*block_args| yield(*block_args) if block_given?
    end

The effect of course depends on the number of associations in the AR model and the number of calls to those associations.

In my case that saves 5 megabytes of memory and 100 ms when updating 100 AR objects (each have 6 associations) and also about the same amount of memory and time when rendering them.

Please see the blog post for details: http://blog.pluron.com/2008/02/rails-faster-as.html

Attachments

no_named_block_arguments_in_associations.patch (4.7 kB) - added by adymo on 02/13/08 20:12:47.
no_named_block_arguments_in_associations_fixed.patch (5.9 kB) - added by adymo on 02/19/08 17:33:40.
Correct patch with a little testcase
no_named_block_arguments_in_associations_fixed_2.patch (5.8 kB) - added by adymo on 02/29/08 14:44:41.
Third version of the patch with all fixes and consistent formatting

Change History

02/13/08 20:12:47 changed by adymo

  • attachment no_named_block_arguments_in_associations.patch added.

02/13/08 23:45:58 changed by jamesh

Hi! I've verified this against 2.0 stable. I'm curious about how this affects overriding methods. Suppose, for instance, I wanted to override "sum". In the past... 

alias_method_chain :sum, :loud_voices

def sum_with_loud_voices(*args, &block)
  logger.info "Wow! I'm overriding a method!!!"
  sum_without_loud_voices(*args, &block)
end

So, without named blocks, how does one override such a method? (Apologies if these sounds a little on the newb side. I'm a little unfamiliar with how you've done blocks in this patch.)

02/14/08 07:22:23 changed by technoweenie

  • owner changed from core to technoweenie.
  • status changed from new to assigned.
alias_method_chain :sum, :loud_voices

def sum_with_loud_voices(*args)
  logger.info "Wow! I'm overriding a method!!!"
  sum_without_loud_voices(*args) { |*block_args| yield(*block_args) if block_given? }
end

(follow-up: ↓ 5 ) 02/14/08 07:24:13 changed by rick

  • status changed from assigned to closed.
  • resolution set to fixed.

(In [8865]) Improve associations performance by avoiding named block arguments. Closes #11109

02/14/08 07:25:00 changed by technoweenie

Argh, apologies for leaving your credit off the log msg, but it's on the CHANGELOG.

(in reply to: ↑ 3 ; follow-up: ↓ 6 ) 02/14/08 13:21:01 changed by pdcawley

Replying to rick:

(In [8865]) Improve associations performance by avoiding named block arguments. Closes #11109

Just a comment on this patch - I know it's bringing the ugly in the name of optimization, but this patch arguably does that already - isn't there something to be gained in not creating long lexical chains of blocks by doing: 

if block_given?
    whatever(*args) {|*block_args| yield(*block_args)}
else
    whatever(*args)
end

In the patch itself, the implementation of any? should probably be: 

def any? 
  if block_given? 
    method_missing(:any?) { |*block_args| yield(*block_args)} 
  else 
    !empty? 
  end 
end

No point evaluating the condition twice after all.

02/19/08 17:33:40 changed by adymo

  • attachment no_named_block_arguments_in_associations_fixed.patch added.

Correct patch with a little testcase

(in reply to: ↑ 5 ) 02/19/08 17:42:58 changed by adymo

  • status changed from closed to reopened.
  • resolution deleted.

Replying to pdcawley:

Just a comment on this patch - I know it's bringing the ugly in the name of optimization, but this patch arguably does that already - isn't there something to be gained in not creating long lexical chains of blocks by doing: No point evaluating the condition twice after all.

You're right. There's no point in passing extra blocks. More to say, those blocks may break because some Array methods (like #any? or #sort) change semantics depending on the block existence.

For example, code like authors(:david).categories.sort would break with my previous patch because method_missing would pass the empty block that returns nothing to the #sort method. Array#sort would then fail to sort records exactly because the block doesn't return the result of comparison between two elements that Array#sort expects.

I've just added the fixed patch and thus I'm reopening the ticket again. With this new patch all AP and AR tests pass on my computer.

02/29/08 04:51:49 changed by nzkoz

Hey Guys,

looks good, but could you fix the formatting to use two spaces (there are a few inconsistencies in there). Also, when we have a two-line block we use do/end not {}

Tests pass and I'm happy to go with it once the cleanup is done.

02/29/08 14:44:41 changed by adymo

  • attachment no_named_block_arguments_in_associations_fixed_2.patch added.

Third version of the patch with all fixes and consistent formatting

02/29/08 23:16:56 changed by nzkoz

  • status changed from reopened to closed.
  • resolution set to fixed.

(In [8957]) Improve performance by avoiding named block arguments. Closes #11109 [adymo]

Reapplies [8865] with some fixes