Skip to content

perf: skip redundant downcase in Currency.find! on cache hits#521

Open
cribbles wants to merge 1 commit intomainfrom
cribbles/perf-cache-currency-resolution
Open

perf: skip redundant downcase in Currency.find! on cache hits#521
cribbles wants to merge 1 commit intomainfrom
cribbles/perf-cache-currency-resolution

Conversation

@cribbles
Copy link
Copy Markdown
Contributor

@cribbles cribbles commented Mar 11, 2026

Summary

Optimize Currency.find! (aliased from Currency.new) to skip to_s.downcase on cache hits. On the first resolution of a currency string (e.g. "USD"), the result is stored under both the original-case and downcased keys in @@loaded_currencies. Subsequent lookups hit the fast path immediately without allocating a downcased string or checking the mutex.

Stacks on #520.

Benchmark (2M iterations of Money.new)
require "money"
require "benchmark"

n = 2_000_000
c = Money::Currency.find!("USD")

Benchmark.bm do |x|
  x.report("string") { n.times { Money.new(100, "USD") } }
  x.report("currency") { n.times { Money.new(100, c) } }
end

main

              user     system      total        real
string    1.801176   0.008822   1.809998 (  1.942647)
currency  1.305507   0.004958   1.310465 (  1.423729)

#520 (base branch)

              user     system      total        real
string    1.735361   0.005490   1.740851 (  1.860185)
currency  1.237422   0.003729   1.241151 (  1.351668)

This branch

              user     system      total        real
string    1.618875   0.007278   1.626153 (  1.797001)
currency  1.200702   0.005228   1.205930 (  1.378954)

Summary

                    main → #520      #520 → this     main → this
string              ~4%              ~7%              ~10%
currency            ~5%              ~3%              ~8%

@cribbles cribbles changed the title perf: cache string-to-Currency mappings in value_to_currency perf: cache string-to-Currency mappings in #value_to_currency Mar 11, 2026
@cribbles cribbles marked this pull request as ready for review March 11, 2026 16:04
@cribbles cribbles requested a review from elfassy March 11, 2026 16:06
Copy link
Copy Markdown
Contributor

@elfassy elfassy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're better off improving the find! method (already cached) than adding another layer of caching. Ex #519 (comment). The only difference is that this PR is not downcasing the currency string. This means the cached hash could be double the size if people use both lower and upper case, making each search slower and use more memory. You can account for the larger hash by looping through every currency in both upcase and lowercase a few times in your benchmark

@cribbles cribbles force-pushed the cribbles/perf-skip-redundant-currency-resolution branch from 1391c5d to 7eb9050 Compare March 13, 2026 16:08
@cribbles cribbles force-pushed the cribbles/perf-cache-currency-resolution branch 3 times, most recently from fdce13e to ca934a4 Compare March 13, 2026 17:03
@cribbles cribbles force-pushed the cribbles/perf-cache-currency-resolution branch from ca934a4 to 3f417f5 Compare March 13, 2026 17:12
@cribbles cribbles changed the title perf: cache string-to-Currency mappings in #value_to_currency perf: skip redundant downcase in Currency.find! on cache hits Mar 13, 2026
Base automatically changed from cribbles/perf-skip-redundant-currency-resolution to main March 16, 2026 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants