We have an application that displays thousands of currency values on a
page, using the "number_to_currency" converter. When profiling the view
rendering time, I found that a large amount of time was spent in the number
helper classes. One level deeper, a bunch of time was spent calling
I18n.translate to retrieve format options that were the same for the entire
page.
I think it would be nice to optimize these helpers to allow subclasses of
NumberConverter to be reused. I spent 10 minutes modifying the converters
to allow them to be called more than once with the same options. After
that, I did a quick test using bench.rb from the attached diff and saw
roughly a 10x improvement in speed:
$ bundle exec ./bench.rb
Rehearsal ------------------------------------------------------
original delimited 3.110000 0.070000 3.180000 ( 3.262467)
reused delimited 0.270000 0.010000 0.280000 ( 0.284962)
original currency 13.140000 0.100000 13.240000 ( 13.395538)
reused currency 1.240000 0.020000 1.260000 ( 1.276842)
-------------------------------------------- total: 17.960000sec
user system total real
original delimited 2.820000 0.010000 2.830000 ( 2.846727)
reused delimited 0.250000 0.010000 0.260000 ( 0.265952)
original currency 12.870000 0.080000 12.950000 ( 13.020219)
reused currency 1.220000 0.020000 1.240000 ( 1.241132)
Are these changes something the Rails Core team is amenable to pursuing?
Should I open a PR with my changes and some additional tests?
Thanks!
-md
--
You received this message because you are subscribed to the Google Groups "Ruby
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to rubyonrails-core+unsubscr...@googlegroups.com.
To post to this group, send email to rubyonrails-core@googlegroups.com.
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
diff --git a/activesupport/bench.rb b/activesupport/bench.rb
new file mode 100755
index 0000000000..4a81de97f2
--- /dev/null
+++ b/activesupport/bench.rb
@@ -0,0 +1,35 @@
+#!/usr/bin/env ruby
+
+require 'benchmark'
+require 'active_support'
+
+count = 100_000
+number = 1_000_000
+
+Benchmark.bmbm do |x|
+ x.report('original delimited') do
+ count.times do
+ ActiveSupport::NumberHelper::NumberToDelimitedConverter.new(number).execute
+ end
+ end
+
+ x.report('reused delimited') do
+ converter = ActiveSupport::NumberHelper::NumberToDelimitedConverter.new
+ count.times do
+ converter.execute(number)
+ end
+ end
+
+ x.report('original currency') do
+ count.times do
+ ActiveSupport::NumberHelper::NumberToCurrencyConverter.new(number).execute
+ end
+ end
+
+ x.report('reused currency') do
+ converter = ActiveSupport::NumberHelper::NumberToCurrencyConverter.new
+ count.times do
+ converter.execute(number)
+ end
+ end
+end
diff --git a/activesupport/lib/active_support/number_helper/number_converter.rb b/activesupport/lib/active_support/number_helper/number_converter.rb
index 5ea9c8f113..b71e75069c 100644
--- a/activesupport/lib/active_support/number_helper/number_converter.rb
+++ b/activesupport/lib/active_support/number_helper/number_converter.rb
@@ -120,16 +120,19 @@ def self.convert(number, options)
new(number, options).execute
end
- def initialize(number, options)
- @number = number
- @opts = options.symbolize_keys
+ def initialize(*args)
+ raise if args.length > 2
+ @number = args.shift unless args.first.is_a?(Hash)
+ @opts = (args.shift || {}).symbolize_keys
end
- def execute
+ def execute(number = self.number)
if !number
nil
elsif validate_float? && !valid_float?
number
+ elsif method(:convert).arity > 0
+ convert(number)
else
convert
end
diff --git a/activesupport/lib/active_support/number_helper/number_to_currency_converter.rb b/activesupport/lib/active_support/number_helper/number_to_currency_converter.rb
index 1943b9e295..385260d17f 100644
--- a/activesupport/lib/active_support/number_helper/number_to_currency_converter.rb
+++ b/activesupport/lib/active_support/number_helper/number_to_currency_converter.rb
@@ -7,8 +7,8 @@ module NumberHelper
class NumberToCurrencyConverter < NumberConverter # :nodoc:
self.namespace = :currency
- def convert
- number = self.number.to_s.strip
+ def convert(number = self.number)
+ number = number.to_s.strip
format = options[:format]
if number.to_f.negative?
diff --git a/activesupport/lib/active_support/number_helper/number_to_delimited_converter.rb b/activesupport/lib/active_support/number_helper/number_to_delimited_converter.rb
index d5b5706705..ae28b6c42c 100644
--- a/activesupport/lib/active_support/number_helper/number_to_delimited_converter.rb
+++ b/activesupport/lib/active_support/number_helper/number_to_delimited_converter.rb
@@ -7,13 +7,13 @@ class NumberToDelimitedConverter < NumberConverter #:nodoc:
DEFAULT_DELIMITER_REGEX = /(\d)(?=(\d\d\d)+(?!\d))/
- def convert
- parts.join(options[:separator])
+ def convert(number = self.number)
+ parts(number).join(options[:separator])
end
private
- def parts
+ def parts(number)
left, right = number.to_s.split(".".freeze)
left.gsub!(delimiter_pattern) do |digit_to_delimit|
"#{digit_to_delimit}#{options[:delimiter]}"
diff --git a/activesupport/lib/active_support/number_helper/number_to_human_converter.rb b/activesupport/lib/active_support/number_helper/number_to_human_converter.rb
index 03eb6671ec..227cc1ccd1 100644
--- a/activesupport/lib/active_support/number_helper/number_to_human_converter.rb
+++ b/activesupport/lib/active_support/number_helper/number_to_human_converter.rb
@@ -10,9 +10,9 @@ class NumberToHumanConverter < NumberConverter # :nodoc:
self.namespace = :human
self.validate_float = true
- def convert # :nodoc:
- @number = RoundingHelper.new(options).round(number)
- @number = Float(number)
+ def convert(number = self.number) # :nodoc:
+ number = RoundingHelper.new(options).round(number)
+ number = Float(number)
# for backwards compatibility with those that didn't add strip_insignificant_zeros to their locale files
unless options.key?(:strip_insignificant_zeros)
@@ -20,11 +20,11 @@ def convert # :nodoc:
end
units = opts[:units]
- exponent = calculate_exponent(units)
- @number = number / (10**exponent)
+ exponent = calculate_exponent(number, units)
+ number = number / (10**exponent)
rounded_number = NumberToRoundedConverter.convert(number, options)
- unit = determine_unit(units, exponent)
+ unit = determine_unit(number, units, exponent)
format.gsub("%n".freeze, rounded_number).gsub("%u".freeze, unit).strip
end
@@ -34,7 +34,7 @@ def format
options[:format] || translate_in_locale("human.decimal_units.format")
end
- def determine_unit(units, exponent)
+ def determine_unit(number, units, exponent)
exp = DECIMAL_UNITS[exponent]
case units
when Hash
@@ -46,7 +46,7 @@ def determine_unit(units, exponent)
end
end
- def calculate_exponent(units)
+ def calculate_exponent(number, units)
exponent = number != 0 ? Math.log10(number.abs).floor : 0
unit_exponents(units).find { |e| exponent >= e } || 0
end
diff --git a/activesupport/lib/active_support/number_helper/number_to_human_size_converter.rb b/activesupport/lib/active_support/number_helper/number_to_human_size_converter.rb
index 842f2fc8df..3694230b90 100644
--- a/activesupport/lib/active_support/number_helper/number_to_human_size_converter.rb
+++ b/activesupport/lib/active_support/number_helper/number_to_human_size_converter.rb
@@ -8,21 +8,21 @@ class NumberToHumanSizeConverter < NumberConverter #:nodoc:
self.namespace = :human
self.validate_float = true
- def convert
- @number = Float(number)
+ def convert(number = self.number)
+ number = Float(number)
# for backwards compatibility with those that didn't add strip_insignificant_zeros to their locale files
unless options.key?(:strip_insignificant_zeros)
options[:strip_insignificant_zeros] = true
end
- if smaller_than_base?
+ if smaller_than_base?(number)
number_to_format = number.to_i.to_s
else
- human_size = number / (base**exponent)
+ human_size = number / (base**exponent(number))
number_to_format = NumberToRoundedConverter.convert(human_size, options)
end
- conversion_format.gsub("%n".freeze, number_to_format).gsub("%u".freeze, unit)
+ conversion_format.gsub("%n".freeze, number_to_format).gsub("%u".freeze, unit(number))
end
private
@@ -31,23 +31,23 @@ def conversion_format
translate_number_value_with_default("human.storage_units.format", locale: options[:locale], raise: true)
end
- def unit
- translate_number_value_with_default(storage_unit_key, locale: options[:locale], count: number.to_i, raise: true)
+ def unit(number)
+ translate_number_value_with_default(storage_unit_key(number), locale: options[:locale], count: number.to_i, raise: true)
end
- def storage_unit_key
- key_end = smaller_than_base? ? "byte" : STORAGE_UNITS[exponent]
+ def storage_unit_key(number)
+ key_end = smaller_than_base?(number) ? "byte" : STORAGE_UNITS[exponent(number)]
"human.storage_units.units.#{key_end}"
end
- def exponent
+ def exponent(number)
max = STORAGE_UNITS.size - 1
exp = (Math.log(number) / Math.log(base)).to_i
exp = max if exp > max # avoid overflow for the highest unit
exp
end
- def smaller_than_base?
+ def smaller_than_base?(number)
number.to_i < base
end
diff --git a/activesupport/lib/active_support/number_helper/number_to_percentage_converter.rb b/activesupport/lib/active_support/number_helper/number_to_percentage_converter.rb
index 4dcdad2e2c..3f8b592417 100644
--- a/activesupport/lib/active_support/number_helper/number_to_percentage_converter.rb
+++ b/activesupport/lib/active_support/number_helper/number_to_percentage_converter.rb
@@ -5,7 +5,7 @@ module NumberHelper
class NumberToPercentageConverter < NumberConverter # :nodoc:
self.namespace = :percentage
- def convert
+ def convert(number = self.number)
rounded_number = NumberToRoundedConverter.convert(number, options)
options[:format].gsub("%n".freeze, rounded_number)
end
diff --git a/activesupport/lib/active_support/number_helper/number_to_phone_converter.rb b/activesupport/lib/active_support/number_helper/number_to_phone_converter.rb
index 96410f4995..e7c5161731 100644
--- a/activesupport/lib/active_support/number_helper/number_to_phone_converter.rb
+++ b/activesupport/lib/active_support/number_helper/number_to_phone_converter.rb
@@ -3,7 +3,7 @@
module ActiveSupport
module NumberHelper
class NumberToPhoneConverter < NumberConverter #:nodoc:
- def convert
+ def convert(number = self.number)
str = country_code(opts[:country_code]).dup
str << convert_to_phone_number(number.to_s.strip)
str << phone_ext(opts[:extension])
diff --git a/activesupport/lib/active_support/number_helper/number_to_rounded_converter.rb b/activesupport/lib/active_support/number_helper/number_to_rounded_converter.rb
index b7ad76bb62..2ae87e20ca 100644
--- a/activesupport/lib/active_support/number_helper/number_to_rounded_converter.rb
+++ b/activesupport/lib/active_support/number_helper/number_to_rounded_converter.rb
@@ -6,7 +6,7 @@ class NumberToRoundedConverter < NumberConverter # :nodoc:
self.namespace = :precision
self.validate_float = true
- def convert
+ def convert(number = self.number)
helper = RoundingHelper.new(options)
rounded_number = helper.round(number)
@@ -37,10 +37,6 @@ def convert
private
- def calculate_rounded_number(multiplier)
- (number / BigDecimal.new(multiplier.to_f.to_s)).round * multiplier
- end
-
def digit_count(number)
number.zero? ? 1 : (Math.log10(absolute_number(number)) + 1).floor
end
