Skip to content

Commit

Permalink
Refactor write attribute logic to convert number column value
Browse files Browse the repository at this point in the history
This is an improvement for issue rails#8673:
    "Comparing a BigDecimal to true/false on write_attribute is slow"

It seems to be an issue with Ruby itself, related to the "coerce" method
being called in TrueClass/FalseClass due to the == condition, triggering
method_missing, then raising a NameError that's later catched.

This issue was also opened in Ruby tracker:
    https://bugs.ruby-lang.org/issues/7645.

This refactoring avoid the coerce call by using a case statement, which
gives us better readability as well. A simple benchmark:

----------

require 'benchmark/ips'
require 'bigdecimal'

Benchmark.ips do |x|
  x.report("== true")   { BigDecimal('3') == true }
  x.report("TrueClass") { TrueClass === BigDecimal('3') }
  x.report("== 0")      { BigDecimal('3') == 0 }
  x.report("Numeric")   { Numeric === BigDecimal('3') }
end

Calculating -------------------------------------
             == true      6427 i/100ms
           TrueClass     47297 i/100ms
                == 0     35923 i/100ms
             Numeric     55530 i/100ms
-------------------------------------------------
             == true    75878.5 (±21.6%) i/s -     359912 in   5.004392s
           TrueClass  1249547.0 (±13.1%) i/s -    6148610 in   5.035964s
                == 0   666856.3 (±13.3%) i/s -    3268993 in   5.013789s
             Numeric  1269300.9 (±11.3%) i/s -    6274890 in   5.028458s

----------

Master has a very different implementation, and there are apparently no
similar conversions at this point, it's mainly delegated to the column
type cast, but I'll check if something needs to be changed there as well.

Closes rails#8673.
  • Loading branch information
carlosantoniodasilva committed Jan 7, 2013
1 parent 3aebe13 commit c75b5a8
Showing 1 changed file with 5 additions and 4 deletions.
9 changes: 5 additions & 4 deletions activerecord/lib/active_record/attribute_methods/write.rb
Expand Up @@ -54,12 +54,13 @@ def type_cast_attribute_for_write(column, value)
end

def convert_number_column_value(value)
if value == false
case value
when FalseClass
0
elsif value == true
when TrueClass
1
elsif value.is_a?(String) && value.blank?
nil
when String
value.presence
else
value
end
Expand Down

0 comments on commit c75b5a8

Please sign in to comment.