diff options
author | Ryan De Villa <ryand@nulogy.com> | 2016-07-18 13:55:15 -0400 |
---|---|---|
committer | Ryan De Villa <ryand@nulogy.com> | 2016-08-23 14:07:07 -0400 |
commit | fa9f1538f034667ea5d55838bd46844642b54274 (patch) | |
tree | ad4ca0608a6fc7362266fbfc90f059dd381d3b28 | |
parent | 58e0b1d6fc5b6762fbe667c24d808d76b57c7914 (diff) | |
download | rails-fa9f1538f034667ea5d55838bd46844642b54274.tar.gz rails-fa9f1538f034667ea5d55838bd46844642b54274.tar.bz2 rails-fa9f1538f034667ea5d55838bd46844642b54274.zip |
Fix performance regression in `TimeWithZone#to_time`
A performance regression was introduced by commit b79adc4323ff289aed3f5787fdfbb9542aa4f89f
from Rails 4.0.0.beta1, in which `TimeWithZone#to_time` no longer returns a
cached instance attribute but instead coerces the value to `Time`. This
coerced value is not cached, and recomputation degrades the performance
of comparisons between TimeWithZone objects.
See https://github.com/rails/rails/commit/b79adc4323ff289aed3f5787fdfbb9542aa4f89f#diff-3497a506c921a3a3e40fd517e92e4fe3R322
for the change in question.
The following benchmark, which reverts the change linked above, demonstrates
the performance regression:
require 'active_support/time'
require 'benchmark/ips'
utc = Time.utc(2000, 1, 1, 0)
time_zone = ActiveSupport::TimeZone['Eastern Time (US & Canada)']
twz = ActiveSupport::TimeWithZone.new(utc, time_zone)
twz2 = ActiveSupport::TimeWithZone.new(Time.utc(1999, 12, 31, 23, 59, 59), ActiveSupport::TimeZone['UTC'])
patchedTimeWithZone = Class.new(ActiveSupport::TimeWithZone) do
def to_time
utc
end
end
patched_twz = patchedTimeWithZone.new(utc, time_zone)
patched_twz2 = patchedTimeWithZone.new(Time.utc(1999, 12, 31, 23, 59, 59), ActiveSupport::TimeZone['UTC'])
Benchmark.ips do |x|
x.report("comparison out of the box") { twz <=> twz2 }
x.report("comparison reverting to_time") { patched_twz <=> patched_twz2 }
x.compare!
end
The results, when run in rails-dev-box, are as follows:
Warming up --------------------------------------
comparison out of the box
24.765k i/100ms
comparison reverting to_time
57.237k i/100ms
Calculating -------------------------------------
comparison out of the box
517.245k (± 4.7%) i/s - 2.600M in 5.038700s
comparison reverting to_time
2.624M (± 5.0%) i/s - 13.050M in 4.985808s
Comparison:
comparison reverting to_time: 2624266.1 i/s
comparison out of the box: 517244.6 i/s - 5.07x slower
The change made to run the benchmark, however, is not possible, as it would
undo the intent to standardize the return value of `to_time` to `Time` in
the system timezone.
Our proposed solution is to restore the caching behaviour of `to_time`
as it existed prior to the change linked above.
Benchmark of our solution:
require 'active_support/time'
require 'benchmark/ips'
patchedTimeWithZone = Class.new(ActiveSupport::TimeWithZone) do
def to_time
@to_time ||= super
end
end
utc = Time.utc(2000, 1, 1, 0)
time_zone = ActiveSupport::TimeZone['Eastern Time (US & Canada)']
twz = ActiveSupport::TimeWithZone.new(utc, time_zone)
twz2 = ActiveSupport::TimeWithZone.new(Time.utc(1999, 12, 31, 23, 59, 59), ActiveSupport::TimeZone['UTC'])
patched_twz = patchedTimeWithZone.new(utc, time_zone)
patched_twz2 = patchedTimeWithZone.new(Time.utc(1999, 12, 31, 23, 59, 59), ActiveSupport::TimeZone['UTC'])
Benchmark.ips do |x|
x.report("TimeWithZone comparison - existing implementation") { twz <=> twz2 }
x.report("TimeWithZone comparison - caching implementation") { patched_twz <=> patched_twz2 }
x.compare!
end
Results in rails-dev-box:
Warming up --------------------------------------
TimeWithZone comparison - existing implementation
26.629k i/100ms
TimeWithZone comparison - caching implementation
59.144k i/100ms
Calculating -------------------------------------
TimeWithZone comparison - existing implementation
489.757k (± 4.2%) i/s - 2.450M in 5.011639s
TimeWithZone comparison - caching implementation
2.802M (± 5.3%) i/s - 13.958M in 4.996116s
Comparison:
TimeWithZone comparison - caching implementation: 2801519.1 i/s
TimeWithZone comparison - existing implementation: 489756.7 i/s - 5.72x slower
-rw-r--r-- | activesupport/lib/active_support/time_with_zone.rb | 6 |
1 files changed, 6 insertions, 0 deletions
diff --git a/activesupport/lib/active_support/time_with_zone.rb b/activesupport/lib/active_support/time_with_zone.rb index 9d1147620c..f91225289d 100644 --- a/activesupport/lib/active_support/time_with_zone.rb +++ b/activesupport/lib/active_support/time_with_zone.rb @@ -84,6 +84,12 @@ module ActiveSupport end alias_method :getlocal, :localtime + def to_time_with_caching + @to_time ||= to_time_without_caching + end + alias_method :to_time_without_caching, :to_time + alias_method :to_time, :to_time_with_caching + # Returns true if the current time is within Daylight Savings Time for the # specified time zone. # |