Permalink
Browse files

Only yield block if given.

  • Loading branch information...
1 parent 2c203a9 commit 4b5f417e63e4cdd7fd6837c10ebaa9dd4860f55e @miloops miloops committed Jun 24, 2010
Showing with 2 additions and 2 deletions.
  1. +2 −2 activerecord/lib/active_record/relation/query_methods.rb
View
4 activerecord/lib/active_record/relation/query_methods.rb
@@ -22,9 +22,9 @@ def #{query_method}(*args, &block)
end
class_eval <<-CEVAL, __FILE__, __LINE__ + 1
- def select(*args, &block)
+ def select(*args)
if block_given?
- to_a.select(&block)
+ to_a.select { |*block_args| yield(*block_args) }
else
new_relation = clone
value = Array.wrap(args.flatten).reject {|x| x.blank? }

10 comments on commit 4b5f417

@neerajdotname
Ruby on Rails member

there is already an if condition * if block_given? * so I would assume that to_a.select(&block) would be called only if block is provided. Is there some special benefit of using yield that I am missing here?

@josevalim
Ruby on Rails member

Neeraj, we found out that instantiating &block in some methods that were executed a lot was hitting performance and appearing on benchmarks. That said, we decided to do these optimizations. Most of them are already applied though!

@neerajdotname
Ruby on Rails member

cool. thanks for the answer.

@miloops

Neeraj, sorry for such a bad commit message, you now get the idea, it's a performance issue defining &block in method arguments.

@KieranP

Any performance info available? i.e. did it speed up development mode a further 50% ? :-D

@miloops

Changes like this have shown 20-30% perf improvement. Don't think this is the case because it's not called in every query, but it's always good doing it the right way.

@KieranP

Impressive. Would be interesting to see Yehuda's RPS benchmarks now with the recent improvements.

@josevalim
Ruby on Rails member

Yehuda's benchmarks only takes into account the request stack in production and no queries are made. So we should not see a huge improvement in those.

@miloops

For an ActiveRecord bench you can run examples/performance.rb in activerecord directory.

You can also keep track of http://gist.github.com/171408 which is a gist I created back when doing Arel integration for GSoC, this gist contains history update on AR performance. It looks like gist only keep last 10 revisions but that gist was created many months ago (almost a year).

@jonsmock

Thanks everyone, this definitely cleared a lot up for me! Makes sense now!

Please sign in to comment.