Skip to content

Commit 217104e

Browse files
committed
Optimize fast_string_to_time
It's slower than it needs to because it contains a workaround for a bug affecting early Ruby 3.2 releases. It also matches against a regexp which isn't necessary given that starting in Ruby 3.2, `Time.new(str)` is strict enough for this purpose. ``` ruby 3.3.3 (2024-06-12 revision f1c7b6f435) +YJIT [arm64-darwin23] Warming up -------------------------------------- bugged 265.827k i/100ms good 394.774k i/100ms Calculating ------------------------------------- bugged 2.759M (± 5.8%) i/s - 13.823M in 5.031296s good 4.128M (± 7.8%) i/s - 20.528M in 5.010538s Comparison: bugged: 2758945.6 i/s good: 4128323.9 i/s - 1.50x faster ``` ```ruby require "bundler/inline" gemfile(true) do source "https://rubygems.org" gem "benchmark-ips" end require 'benchmark/ips' ISO_DATETIME = / \A (\d{4})-(\d\d)-(\d\d)(?:T|\s) # 2020-06-20T (\d\d):(\d\d):(\d\d)(?:\.(\d{1,6})\d*)? # 10:20:30.123456 (?:(Z(?=\z)|[+-]\d\d)(?::?(\d\d))?)? # +09:00 \z /x def old(string) return unless ISO_DATETIME.match?(string) ::Time.at(::Time.new(string, in: "UTC")) end def fast(string) return unless string.include?("-") # Time.new("1234") # => 1234-01-01 00:00:00 ::Time.new(string, in: "UTC") end str = "2024-01-01T12:43:13" Benchmark.ips do |x| x.report("bugged") { old(str) } x.report("good") { fast(str) } x.compare!(order: :baseline) end ```
1 parent e4f67a6 commit 217104e

File tree

2 files changed

+31
-12
lines changed

2 files changed

+31
-12
lines changed

activemodel/lib/active_model/type/helpers/time_value.rb

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -70,18 +70,33 @@ def new_time(year, mon, mday, hour, min, sec, microsec, offset = nil)
7070
/x
7171

7272
if RUBY_VERSION >= "3.2"
73-
def fast_string_to_time(string)
74-
return unless ISO_DATETIME.match?(string)
75-
76-
if is_utc?
77-
# XXX: Wrapping the Time object with Time.at because Time.new with `in:` in Ruby 3.2.0 used to return an invalid Time object
78-
# see: https://bugs.ruby-lang.org/issues/19292
79-
::Time.at(::Time.new(string, in: "UTC"))
80-
else
81-
::Time.new(string)
73+
if Time.new(2000, 1, 1, 0, 0, 0, "-00:00").yday != 1 # Early 3.2.x had a bug
74+
# BUG: Wrapping the Time object with Time.at because Time.new with `in:` in Ruby 3.2.0
75+
# used to return an invalid Time object
76+
# see: https://bugs.ruby-lang.org/issues/19292
77+
def fast_string_to_time(string)
78+
return unless string.include?("-") # Time.new("1234") # => 1234-01-01 00:00:00
79+
80+
if is_utc?
81+
::Time.at(::Time.new(string, in: "UTC"))
82+
else
83+
::Time.new(string)
84+
end
85+
rescue ArgumentError
86+
nil
87+
end
88+
else
89+
def fast_string_to_time(string)
90+
return unless string.include?("-") # Time.new("1234") # => 1234-01-01 00:00:00
91+
92+
if is_utc?
93+
::Time.new(string, in: "UTC")
94+
else
95+
::Time.new(string)
96+
end
97+
rescue ArgumentError
98+
nil
8299
end
83-
rescue ArgumentError
84-
nil
85100
end
86101
else
87102
def fast_string_to_time(string)

activemodel/lib/active_model/type/helpers/timezone.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,11 @@ module Type
77
module Helpers # :nodoc: all
88
module Timezone
99
def is_utc?
10-
::Time.zone_default.nil? || ::Time.zone_default.match?("UTC")
10+
if default = ::Time.zone_default
11+
default.name == "UTC"
12+
else
13+
true
14+
end
1115
end
1216

1317
def default_timezone

0 commit comments

Comments
 (0)