[PATCH] optimization to AR::Base.exists?

AR::Base exists? speedup

+1 Awesome

+1

I modified Josh's implementation a little and benchmarked it. The performance is slightly better. But ignorable I feel. Still uploading the results anyways so that others can test as well on different platform/db. My results are on Intel Based Mac/MySQL 5.0.x

Thanks.

+1

Benchmarking for various implementations

My version was faster because I removed the rescue block. So +1 for original patch.

All tests pass on postgresql and there is a tiny performance improvement: http://pastie.caboo.se/78987

Replying to lifofifo:

My version was faster because I removed the rescue block. So +1 for original patch.

In Lifo's Implementation the syntax is a bit awkward. However it is faster, and this is an optimization patch.

I could see someone submitting a refactor patch saying "Just use find(:first..) instead" because they have no idea why its that way. Maybe a nice comment that says "!find(:all, ..., :limit => 1).empty? is faster than find(:first)" will help clear things up.

Replying to josh:

In Lifo's Implementation the syntax is a bit awkward. However it is faster, and this is an optimization patch.

Well, Lifo's version misses the rescue clause, so it is not functionally equivalent. I'm not sure why exists? needs the rescue, though my guess is that it catches bogus conditions and someone thought that was necessary. Lifo himself said to use my version after I pointed that out to him.

This is piece of code is of such minor impact that it's not worth debating. It's just low-hanging fruit to do it my way, and we shouldn't waste time wondering if we can get another 1% optimization by getting rid of the rescue clause and not break anything.

Replying to hasmanyjosh:

I'm not sure why exists? needs the rescue, though my guess is that it catches bogus conditions and someone thought that was necessary.

Because find(id) raises a AR::NotFound (so it can show a 404). find(:all) and find(:first) just return false if it can't be found.

Replying to josh:

Because find(id) raises a AR::NotFound (so it can show a 404). find(:all) and find(:first) just return false if it can't be found.

Look at my actual patch. It uses find(:first) too. The only real difference between my patch and Lifo's is the rescue clause.

I guess you shouldn't need it.

>> Page.find(999)
ActiveRecord::RecordNotFound: Couldn't find Page with ID=999
>> Page.find(:first, :conditions => "id = 999")
=> nil
>> Page.find(:all, :conditions => "id = 999", :limit => 1)
=> []

Nevermind anything I've said... its a rescue for ActiveRecordError.

This is why the rescue is this:

------------------------------------------------------------------------
r983 | david | 2005-03-23 02:57:38 +0200 (K, 23 märts 2005) | 1 line

Fixed that AR exists?() would explode on postgresql if the passed id did not match the PK type #900 [Scott Barron]
------------------------------------------------------------------------

# svn diff -r 982:983 base.rb
Index: base.rb
===================================================================
--- base.rb	(revision 982)
+++ base.rb	(revision 983)
@@ -330,7 +330,7 @@
       # Example:
       #   Person.exists?(5)
       def exists?(id)
-        !find_first("#{primary_key} = #{sanitize(id)}").nil?
+        !find_first("#{primary_key} = #{sanitize(id)}").nil? rescue false
       end
 
       # This method is deprecated in favor of find with the :conditions option.

With postgresql 8.2.4 this does not seem to be a problem, not sure even if this is a problem with postgresql itself or the postgresql adapter, but to me this does not seem to be a good way to fix such an issue.

use count instead of find for further performance gains

i've uploaded a patch that i created before i stumbled upon this ticket tonight. i ran it by josh as well. i believe using count may be better suited for performance gains here.

also added some simple unit tests for the exists method.

Looks like the count approach is actually slower. My benchmarks:

                     user     system      total        real
exists?         16.110000   1.510000  17.620000 ( 24.378860)  # original
exists_opt?     15.050000   1.500000  16.550000 ( 21.901476)  # simple optimization
exists_count?   17.930000   1.530000  19.460000 ( 24.787894)  # count optimization

agreed (after some benchmarking)

although, my tests did show that the count method was quicker than the original.

either way, +1 for josh's patch.

fixed in [7274]