Permalink
Browse files

Refactor write attribute logic to convert number column value

This is an improvement for issue #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 #8673.
  • Loading branch information...
1 parent 3aebe13 commit c75b5a88a82c79fdf95dfea4d72bf3c5a829930e @carlosantoniodasilva carlosantoniodasilva committed Jan 7, 2013
Showing with 5 additions and 4 deletions.
  1. +5 −4 activerecord/lib/active_record/attribute_methods/write.rb
@@ -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

0 comments on commit c75b5a8

Please sign in to comment.