Skip to content

perf: skip redundant currency resolution in Money#initialize#520

Merged
cribbles merged 1 commit intomainfrom
cribbles/perf-skip-redundant-currency-resolution
Mar 16, 2026
Merged

perf: skip redundant currency resolution in Money#initialize#520
cribbles merged 1 commit intomainfrom
cribbles/perf-skip-redundant-currency-resolution

Conversation

@cribbles
Copy link
Contributor

@cribbles cribbles commented Mar 11, 2026

Summary

Money.new resolves the currency via Helpers.value_to_currency before calling initialize, but initialize called value_to_currency again on the already-resolved Currency object. This removes the redundant call. init_with (YAML deserialization) is updated to resolve before calling initialize.

~4% faster for string currency codes, ~6% faster for Currency objects.

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.840458   0.004621   1.845079 (  1.845461)
currency  1.354037   0.002053   1.356090 (  1.356246)

This branch

              user     system      total        real
string    1.772362   0.002751   1.775113 (  1.775356)
currency  1.267099   0.002948   1.270047 (  1.270183)

@cribbles cribbles changed the title perf: skip redundant currency resolution in Money#initialize perf: skip redundant currency resolution in Money#initialize 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
@elfassy
Copy link
Contributor

elfassy commented Mar 12, 2026

please update the RBS signatures, otherwise this looks good 👍

@cribbles cribbles force-pushed the cribbles/perf-skip-redundant-currency-resolution branch from 1391c5d to 7eb9050 Compare March 13, 2026 16:08
@cribbles
Copy link
Contributor Author

@elfassy could you clarify what needs to be changed, RBS-wise? steep check didn't uncover anything.

@elfassy
Copy link
Contributor

elfassy commented Mar 13, 2026

I thought the signature accepted a string, but it looks like it was already a Currency object

def initialize: (BigDecimal value, Currency | NullCurrency currency) -> void

@cribbles cribbles merged commit c201b50 into main Mar 16, 2026
9 checks passed
@cribbles cribbles deleted the cribbles/perf-skip-redundant-currency-resolution branch 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