daddyz / phonelib

Ruby gem for phone validation and formatting using google libphonenumber library data
MIT License
1.08k stars 132 forks source link

Memoize regexp inside PhoneAnalyzer#country_code_candidates_for #317

Closed tgwizard closed 5 days ago

tgwizard commented 1 month ago

This code path appears in production profiles for us. Local benchmarks shows that It's >80x faster if we memoize the regexp.

Benchmark ``` ruby 3.3.5 (2024-09-03 revision ef084cc8f4) +YJIT [arm64-darwin23] Warming up -------------------------------------- original_country_code_candidates_for 1.491k i/100ms cached_country_code_candidates_for 74.288k i/100ms smarter_cached_country_code_candidates_for 109.646k i/100ms Calculating ------------------------------------- original_country_code_candidates_for 12.539k (± 8.2%) i/s (79.75 μs/i) - 62.622k in 5.045558s cached_country_code_candidates_for 751.572k (± 4.9%) i/s (1.33 μs/i) - 3.789M in 5.054955s smarter_cached_country_code_candidates_for 1.087M (± 2.3%) i/s (920.34 ns/i) - 5.482M in 5.048195s Comparison: smarter_cached_country_code_candidates_for: 1086552.9 i/s cached_country_code_candidates_for: 751571.7 i/s - 1.45x slower original_country_code_candidates_for: 12539.3 i/s - 86.65x slower ``` ```ruby PHONE = "+4623123123" def original_country_code_candidates_for(phone) stripped_phone = phone.gsub(/^(#{Phonelib.phone_data_int_prefixes})/, "") ((1..3).map { |length| phone[0, length] } + (1..3).map { |length| stripped_phone[0, length] }).uniq end def cached_country_code_candidates_for(phone) raw_re = "^(#{Phonelib.phone_data_int_prefixes})" re = Phonelib.phone_regexp_cache[raw_re] ||= Regexp.new(raw_re).freeze stripped_phone = phone.gsub(re, "") ((1..3).map { |length| phone[0, length] } + (1..3).map { |length| stripped_phone[0, length] }).uniq end def smarter_cached_country_code_candidates_for(phone) re = Phonelib.phone_regexp_cache["phone_data_int_prefixes"] ||= Regexp.new("^(#{Phonelib.phone_data_int_prefixes})").freeze stripped_phone = phone.gsub(re, "") ((1..3).map { |length| phone[0, length] } + (1..3).map { |length| stripped_phone[0, length] }).uniq end Benchmark.ips do |x| x.report("original_country_code_candidates_for") { original_country_code_candidates_for(PHONE) } x.report("cached_country_code_candidates_for") { cached_country_code_candidates_for(PHONE) } x.report("smarter_cached_country_code_candidates_for") { smarter_cached_country_code_candidates_for(PHONE) } x.compare! end ```
tgwizard commented 2 weeks ago

cc @daddyz