From c91b730122cd0ce98f1508c273cd671627729177 Mon Sep 17 00:00:00 2001 From: wps260 <278816799+wps260@users.noreply.github.com> Date: Wed, 29 Apr 2026 14:47:01 -0500 Subject: [PATCH] Performance improvements in balance sync cache (#1581) * Performance improvements in balance sync cache Balance::SyncCache#converted_holdings called account.holdings.map { |h| h.dup } which duplicated every holding record into a new ActiveRecord object, converted its currency, and stored the full object in a holdings_by_date array hash. For an investment account with years of history this allocates 100,000+ AR objects on every sync - one per holding row - creating proportional GC pressure that scaled with account age. The only consumer of get_holdings(date) was BaseCalculator#holdings_value_for_date, which immediately discarded the objects after calling .sum(&:amount). The individual holding objects were never accessed for any other attribute. Replace the dup-and-group approach with a single aggregation pass that stores only the per-date sum: holdings_value_by_date: account.holdings.each_with_object(Hash.new(0)) do |h, totals| converted = Money.new(h.amount, h.currency).exchange_to(account.currency, date: h.date).amount totals[h.date] += converted end Interface change: get_holdings(date) -> get_holdings_value(date) returns a Numeric directly rather than an Array. BaseCalculator#holdings_value_for_date is updated accordingly, and its own per-date memoization layer is removed since holdings_value_by_date is already fully memoized at the SyncCache level. * fall back to 1:1 rate in SyncCache when holding exchange rate is missing; update tests to use investment class --- app/models/balance/base_calculator.rb | 3 +- app/models/balance/sync_cache.rb | 27 ++++++--------- test/models/balance/sync_cache_test.rb | 46 +++++++++++++++++++++++++- 3 files changed, 57 insertions(+), 19 deletions(-) diff --git a/app/models/balance/base_calculator.rb b/app/models/balance/base_calculator.rb index a1e43d99e..c8b090547 100644 --- a/app/models/balance/base_calculator.rb +++ b/app/models/balance/base_calculator.rb @@ -15,8 +15,7 @@ class Balance::BaseCalculator end def holdings_value_for_date(date) - @holdings_value_for_date ||= {} - @holdings_value_for_date[date] ||= sync_cache.get_holdings(date).sum(&:amount) + sync_cache.get_holdings_value(date) end def derive_cash_balance_on_date_from_total(total_balance:, date:) diff --git a/app/models/balance/sync_cache.rb b/app/models/balance/sync_cache.rb index 582774c38..1f1a93a68 100644 --- a/app/models/balance/sync_cache.rb +++ b/app/models/balance/sync_cache.rb @@ -7,8 +7,8 @@ class Balance::SyncCache entries_by_date[date]&.find { |e| e.valuation? } end - def get_holdings(date) - holdings_by_date[date] || [] + def get_holdings_value(date) + holdings_value_by_date[date] || 0 end def get_entries(date) @@ -22,8 +22,15 @@ class Balance::SyncCache @entries_by_date ||= converted_entries.group_by(&:date) end - def holdings_by_date - @holdings_by_date ||= converted_holdings.group_by(&:date) + def holdings_value_by_date + @holdings_value_by_date ||= account.holdings.each_with_object(Hash.new(0)) do |h, totals| + begin + converted = Money.new(h.amount, h.currency).exchange_to(account.currency, date: h.date).amount + rescue Money::ConversionError + converted = h.amount # fallback to 1:1 conversion rate if exchange rate unavailable + end + totals[h.date] += converted + end end def converted_entries @@ -46,16 +53,4 @@ class Balance::SyncCache converted_entry end end - - def converted_holdings - @converted_holdings ||= account.holdings.map do |h| - converted_holding = h.dup - converted_holding.amount = converted_holding.amount_money.exchange_to( - account.currency, - date: h.date - ).amount - converted_holding.currency = account.currency - converted_holding - end - end end diff --git a/test/models/balance/sync_cache_test.rb b/test/models/balance/sync_cache_test.rb index 0edf1d30d..775a730d3 100644 --- a/test/models/balance/sync_cache_test.rb +++ b/test/models/balance/sync_cache_test.rb @@ -5,7 +5,7 @@ class Balance::SyncCacheTest < ActiveSupport::TestCase @family = families(:dylan_family) @account = @family.accounts.create!( name: "Test Account", - accountable: Depository.new, + accountable: Investment.new, currency: "USD", balance: 1000 ) @@ -125,6 +125,50 @@ class Balance::SyncCacheTest < ActiveSupport::TestCase assert_in_delta 120.0, amounts[2], 0.01 # 100 EUR * 1.2 end + # get_holdings_value + + test "returns 0 for date with no holdings" do + cache = Balance::SyncCache.new(@account) + assert_equal 0, cache.get_holdings_value(Date.current) + end + + test "sums holdings value for a single date" do + security = Security.create!(ticker: "TST", name: "Test") + + @account.holdings.create!(security: security, date: Date.current, qty: 10, price: 100, amount: 1000, currency: "USD") + @account.holdings.create!(security: security, date: 1.day.ago.to_date, qty: 10, price: 90, amount: 900, currency: "USD") + + cache = Balance::SyncCache.new(@account) + assert_equal 1000, cache.get_holdings_value(Date.current) + assert_equal 900, cache.get_holdings_value(1.day.ago.to_date) + end + + test "sums multiple holdings on the same date" do + s1 = Security.create!(ticker: "S1", name: "Security 1") + s2 = Security.create!(ticker: "S2", name: "Security 2") + + @account.holdings.create!(security: s1, date: Date.current, qty: 10, price: 100, amount: 1000, currency: "USD") + @account.holdings.create!(security: s2, date: Date.current, qty: 5, price: 200, amount: 1000, currency: "USD") + + assert_equal 2000, Balance::SyncCache.new(@account).get_holdings_value(Date.current) + end + + test "converts foreign currency holdings to account currency" do + ExchangeRate.create!(from_currency: "EUR", to_currency: "USD", date: Date.current, rate: 1.5) + + security = Security.create!(ticker: "TST", name: "Test") + @account.holdings.create!(security: security, date: Date.current, qty: 1, price: 100, amount: 100, currency: "EUR") + + assert_equal 150.0, Balance::SyncCache.new(@account).get_holdings_value(Date.current) + end + + test "falls back to 1:1 conversion rate when exchange rate is missing for a foreign currency holding" do + security = Security.create!(ticker: "TST", name: "Test") + @account.holdings.create!(security: security, date: Date.current, qty: 1, price: 100, amount: 100, currency: "EUR") + + assert_equal 100, Balance::SyncCache.new(@account).get_holdings_value(Date.current) + end + test "prioritizes custom rate over fetched rate" do # Create fetched rate ExchangeRate.create!(