From 3727e8c622d6c382d4215b78973655ba2cf4fdaf Mon Sep 17 00:00:00 2001 From: Kyle Date: Thu, 12 Nov 2026 22:58:54 +0700 Subject: [PATCH] Cache Utils.underscore/camelize results to cut allocations (#125) Utils.underscore (3 gsub! + tr!) ran twice for every JSON key of every response, and camelize ran for every attribute on to_hash (and therefore on == and hash). The set of attribute names is small and finite, so: * Memoize underscore and camelize in a thread-safe, size-capped cache (copy-on-write frozen Hash behind a Mutex: lock-free reads, no new runtime dependency). Results are now frozen. * Skip the regexes for names that are already underscored. * Add Utils.underscore_symbols, which caches the attribute name and setter symbols together, and use it in Resource so that instantiate_record_from_json no longer underscores each key twice nor builds a setter string per key. * Add specs (edge cases, idempotency, thread safety, from_json round trip) and benchmark/utils_benchmark.rb. Co-Authored-By: Claude Sonnet 5.5 --- benchmark/utils_benchmark.rb | 129 +++++++++++++++++++++++ lib/killbill_client/models/resource.rb | 8 +- lib/killbill_client/utils.rb | 116 ++++++++++++++++++-- spec/killbill_client/resource_spec.rb | 35 +++++++ spec/killbill_client/utils_spec.rb | 140 +++++++++++++++++++++++++ 5 files changed, 413 insertions(+), 15 deletions(-) create mode 100644 benchmark/utils_benchmark.rb create mode 100644 spec/killbill_client/utils_spec.rb diff --git a/benchmark/utils_benchmark.rb b/benchmark/utils_benchmark.rb new file mode 100644 index 0000000..b96eda3 --- /dev/null +++ b/benchmark/utils_benchmark.rb @@ -0,0 +1,129 @@ +# Micro-benchmark for the JSON key conversions done on every Kill Bill response. +# +# gem install benchmark-ips +# ruby benchmark/utils_benchmark.rb +# +# It reports iterations/second (benchmark-ips) and objects allocated per iteration for: +# * Utils.underscore / Utils.camelize on real Kill Bill attribute names +# * Resource.from_json / Resource#to_hash on an account-timeline-like payload +# +# To compare against another revision, run it from a checkout of that revision (the script only uses APIs that +# exist on master) and compare the outputs. + +$LOAD_PATH.unshift File.expand_path('../../lib', __FILE__) + +require 'json' +require 'killbill_client' + +begin + require 'benchmark/ips' +rescue LoadError + abort 'benchmark-ips is required: gem install benchmark-ips' +end + +# Attribute names as they appear on the wire (a representative sample of the models in lib/killbill_client/models/gen) +KEYS = %w( + accountId name firstNameLength externalKey email billCycleDayLocal currency parentAccountId isPaymentDelegatedToParent + paymentMethodId referenceTime timeZone address1 address2 postalCode company city state country locale phone notes + isMigrated accountBalance accountCBA auditLogs invoiceItemId invoiceId linkedInvoiceItemId childAccountId bundleId + subscriptionId productName planName phaseName usageName prettyProductName prettyPlanName prettyPhaseName + prettyUsageName itemType description startDate endDate amount rate quantity itemDetails catalogEffectiveDate + childItems invoiceDate targetDate invoiceNumber balance bundleKeys creditAdj refundAdj credits items trackingIds + isParentInvoice parentInvoiceId eventId billingPeriod effectiveDate plan product priceList eventType + isBlockedBilling isBlockedEntitlement serviceName serviceStateName phase paymentId paymentNumber paymentExternalKey + authAmount capturedAmount purchasedAmount refundedAmount creditedAmount transactions paymentAttempts + targetInvoiceId changeType changeDate +).freeze + +SNAKE_KEYS = KEYS.map { |key| KillBillClient::Utils.underscore(key).dup }.freeze + +def allocations + GC.stat[:total_allocated_objects] +end + +# Objects allocated per call, or nil when the runtime does not expose the counter (e.g. JRuby) +def allocations_per_call(iterations = 200) + before = allocations + return nil if before.nil? + + iterations.times { yield } + (allocations - before) / iterations.to_f +end + +def report(label, &block) + # Warm up (also fills the caches, if any), then measure + 50.times(&block) + allocated = allocations_per_call(&block) + puts format('%-34s %s', label, allocated ? format('%.1f objects/iteration', allocated) : 'allocations: n/a on this runtime') +end + +def timeline_json(accounts_items = 40) + audit = [{ 'changeType' => 'INSERT', 'changeDate' => '2024-01-01T00:00:00.000Z', 'objectType' => 'INVOICE', + 'changedBy' => 'admin', 'reasonCode' => nil, 'comments' => nil, 'userToken' => 'token' }] + item = lambda do |i| + { 'invoiceItemId' => "item-#{i}", 'invoiceId' => 'inv', 'accountId' => 'acc', 'bundleId' => 'b', 'subscriptionId' => 's', + 'productName' => 'Standard', 'planName' => 'standard-monthly', 'phaseName' => 'standard-monthly-evergreen', + 'prettyProductName' => 'Standard', 'prettyPlanName' => 'Standard Monthly', 'prettyPhaseName' => 'Standard Monthly Evergreen', + 'itemType' => 'RECURRING', 'startDate' => '2024-01-01', 'endDate' => '2024-02-01', 'amount' => 10.0, 'rate' => 10.0, + 'currency' => 'USD', 'quantity' => 1, 'auditLogs' => audit } + end + invoice = lambda do |i| + { 'amount' => 10.0, 'currency' => 'USD', 'status' => 'COMMITTED', 'creditAdj' => 0.0, 'refundAdj' => 0.0, + 'invoiceId' => "inv-#{i}", 'invoiceDate' => '2024-01-01', 'targetDate' => '2024-01-01', 'invoiceNumber' => i.to_s, + 'balance' => 0.0, 'accountId' => 'acc', 'bundleKeys' => 'k', 'credits' => [], 'items' => Array.new(4) { |j| item.call(j) }, + 'isParentInvoice' => false, 'auditLogs' => audit } + end + bundle = lambda do |i| + { 'accountId' => 'acc', 'bundleId' => "b-#{i}", 'externalKey' => "key-#{i}", 'auditLogs' => audit, + 'subscriptions' => [{ 'accountId' => 'acc', 'bundleId' => "b-#{i}", 'subscriptionId' => "s-#{i}", 'externalKey' => "key-#{i}", + 'startDate' => '2024-01-01', 'productName' => 'Standard', 'productCategory' => 'BASE', + 'billingPeriod' => 'MONTHLY', 'phaseType' => 'EVERGREEN', 'priceList' => 'DEFAULT', + 'planName' => 'standard-monthly', 'state' => 'ACTIVE', 'sourceType' => 'NATIVE', + 'billingStartDate' => '2024-01-01', 'billCycleDayLocal' => 1, 'auditLogs' => audit, + 'events' => Array.new(3) { |j| { 'eventId' => "e-#{j}", 'billingPeriod' => 'MONTHLY', + 'effectiveDate' => '2024-01-01', 'plan' => 'standard-monthly', + 'product' => 'Standard', 'priceList' => 'DEFAULT', + 'eventType' => 'START_ENTITLEMENT', 'isBlockedBilling' => false, + 'isBlockedEntitlement' => false, 'serviceName' => 'entitlement-service', + 'serviceStateName' => 'ENT_STARTED', 'phase' => 'standard-monthly-evergreen', + 'auditLogs' => audit } } }] } + end + payment = lambda do |i| + { 'accountId' => 'acc', 'paymentId' => "p-#{i}", 'paymentNumber' => i.to_s, 'paymentExternalKey' => "pk-#{i}", + 'authAmount' => 0, 'capturedAmount' => 10.0, 'purchasedAmount' => 10.0, 'refundedAmount' => 0, 'creditedAmount' => 0, + 'currency' => 'USD', 'paymentMethodId' => 'pm', 'transactions' => [], 'auditLogs' => audit } + end + + { + 'account' => { 'accountId' => 'acc', 'name' => 'John Doe', 'externalKey' => 'ext', 'email' => 'john@example.com', + 'billCycleDayLocal' => 0, 'currency' => 'USD', 'isPaymentDelegatedToParent' => false, + 'timeZone' => 'UTC', 'country' => 'US', 'isMigrated' => false, 'accountBalance' => 0, 'accountCBA' => 0, + 'auditLogs' => audit }, + 'bundles' => Array.new(accounts_items / 4) { |i| bundle.call(i) }, + 'invoices' => Array.new(accounts_items) { |i| invoice.call(i) }, + 'payments' => Array.new(accounts_items) { |i| payment.call(i) } + }.to_json +end + +JSON_PAYLOAD = timeline_json.freeze + +puts "#{RUBY_ENGINE} #{RUBY_VERSION} (#{RUBY_DESCRIPTION})" +puts "#{KEYS.size} distinct attribute names; timeline payload: #{JSON_PAYLOAD.bytesize} bytes" +puts + +puts 'Allocations' +report('underscore (all keys)') { KEYS.each { |key| KillBillClient::Utils.underscore(key) } } +report('camelize :lower (all keys)') { SNAKE_KEYS.each { |key| KillBillClient::Utils.camelize(key, :lower) } } +report('from_json (account timeline)') { KillBillClient::Model::Resource.from_json(KillBillClient::Model::AccountTimeline, JSON_PAYLOAD) } +TIMELINE = KillBillClient::Model::Resource.from_json(KillBillClient::Model::AccountTimeline, JSON_PAYLOAD) +report('to_hash (account timeline)') { TIMELINE.to_hash } +puts + +Benchmark.ips do |x| + x.config(time: 5, warmup: 2) + + x.report('underscore (all keys)') { KEYS.each { |key| KillBillClient::Utils.underscore(key) } } + x.report('camelize :lower (all keys)') { SNAKE_KEYS.each { |key| KillBillClient::Utils.camelize(key, :lower) } } + x.report('from_json (account timeline)') { KillBillClient::Model::Resource.from_json(KillBillClient::Model::AccountTimeline, JSON_PAYLOAD) } + x.report('to_hash (account timeline)') { TIMELINE.to_hash } +end diff --git a/lib/killbill_client/models/resource.rb b/lib/killbill_client/models/resource.rb index c752f20..eccc723 100644 --- a/lib/killbill_client/models/resource.rb +++ b/lib/killbill_client/models/resource.rb @@ -19,7 +19,7 @@ def initialize(hash = nil) # Make sure we support ActiveSupport::HashWithIndifferentAccess for Kaui if hash.respond_to?(:each) hash.each do |key, value| - send("#{Utils.underscore key.to_s}=", value) + send(Utils.underscore_symbols(key).last, value) end end end @@ -145,12 +145,12 @@ def instantiate_record_from_json(resource_class, data) kb_ancestors = resource_class.ancestors.select { |ancestor| !@@attribute_names[ancestor.name].nil? } data.each do |name, value| - name = Utils.underscore name + name, setter = Utils.underscore_symbols name attr_desc = nil # Allow for inheritance kb_ancestors.each do |ancestor| - attr_desc = @@attribute_names[ancestor.name][name.to_sym] + attr_desc = @@attribute_names[ancestor.name][name] break unless attr_desc.nil? end @@ -172,7 +172,7 @@ def instantiate_record_from_json(resource_class, data) end #end unless attr_desc.nil? or data_elem.blank? # TODO Be lenient for now to support different API formats - record.send("#{Utils.underscore name}=", value) rescue nil + record.send(setter, value) rescue nil end #end data.each diff --git a/lib/killbill_client/utils.rb b/lib/killbill_client/utils.rb index 88c600d..abe9a70 100644 --- a/lib/killbill_client/utils.rb +++ b/lib/killbill_client/utils.rb @@ -2,7 +2,108 @@ module KillBillClient module Utils ACRONYMS = %w(CBA).freeze + # Small thread-safe, size-bounded memoization table. + # + # Reads are lock-free: the table is an immutable (frozen) Hash which is swapped atomically on writes + # (copy-on-write, guarded by a Mutex). The set of keys we see in practice is tiny and finite (the attribute names + # of the Kill Bill JSON API), so the cost of copying on a miss is negligible, and the size cap is only a safety + # net: once it is reached, new keys are simply computed on each call instead of being cached. + # + # Values must never be nil or false. + class Cache + DEFAULT_MAX_SIZE = 4096 + + def initialize(max_size = DEFAULT_MAX_SIZE) + @max_size = max_size + @mutex = Mutex.new + @entries = {}.freeze + end + + def fetch(key) + value = @entries[key] + return value unless value.nil? + + value = yield + store(key, value) + value + end + + def size + @entries.size + end + + private + + def store(key, value) + @mutex.synchronize do + entries = @entries + return if entries.size >= @max_size || entries.key?(key) + + updated = entries.dup + updated[key] = value + @entries = updated.freeze + end + end + end + + UNDERSCORE_CACHE = Cache.new + UNDERSCORE_SYMBOLS_CACHE = Cache.new + CAMELIZE_UPPER_CACHE = Cache.new + CAMELIZE_LOWER_CACHE = Cache.new + + # Anything that is not already in underscored form: uppercase letters (including non-ASCII ones, which + # String#downcase! would lowercase), dashes and namespace separators. + NEEDS_UNDERSCORE = /[[:upper:]]|-|::/ + + # @return [String] a frozen string def camelize(underscored_word, first_letter = :upper) + cache = first_letter == :lower ? CAMELIZE_LOWER_CACHE : CAMELIZE_UPPER_CACHE + return compute_camelize(underscored_word, first_letter).freeze unless cacheable?(underscored_word) + + cache.fetch(underscored_word) { compute_camelize(underscored_word, first_letter).freeze } + end + + def demodulize(class_name_in_module) + class_name_in_module.to_s.sub(/^.*::/, '') + end + + # @return [String] a frozen string + def underscore(camel_cased_word) + return compute_underscore(camel_cased_word).freeze unless cacheable?(camel_cased_word) + + UNDERSCORE_CACHE.fetch(camel_cased_word) { compute_underscore(camel_cased_word).freeze } + end + + # Resolves, in one cached lookup, what is needed to map a JSON attribute name onto a resource attribute. + # + # @return [Array] a frozen [attribute_name, setter_name] pair, e.g. [:account_cba, :account_cba=] for 'accountCBA' + def underscore_symbols(camel_cased_word) + unless cacheable?(camel_cased_word) + name = underscore(camel_cased_word).to_sym + return [name, :"#{name}="].freeze + end + + UNDERSCORE_SYMBOLS_CACHE.fetch(camel_cased_word) do + name = underscore(camel_cased_word).to_sym + [name, :"#{name}="].freeze + end + end + + def acronym?(word) + ACRONYMS.include?(word.to_s.upcase) + end + + extend self + + private + + # Only immutable-by-convention keys are memoized: Strings are copied and frozen by Hash on insertion, and Symbols + # are immutable. Anything else (nil, Integer, arbitrary objects with a to_s) is simply computed. + def cacheable?(value) + value.is_a?(String) || value.is_a?(Symbol) + end + + def compute_camelize(underscored_word, first_letter) camelized = underscored_word.to_s.split('_').map do |word| if acronym?(word) word.upcase @@ -14,12 +115,11 @@ def camelize(underscored_word, first_letter = :upper) camelized end - def demodulize(class_name_in_module) - class_name_in_module.to_s.sub(/^.*::/, '') - end - - def underscore(camel_cased_word) + def compute_underscore(camel_cased_word) word = camel_cased_word.to_s.dup + # Fast path: already underscored, nothing to rewrite + return word unless word =~ NEEDS_UNDERSCORE + word.gsub!(/::/, '/') word.gsub!(/([A-Z]+)([A-Z][a-z])/, '\1_\2') word.gsub!(/([a-z\d])([A-Z])/, '\1_\2') @@ -27,11 +127,5 @@ def underscore(camel_cased_word) word.downcase! word end - - def acronym?(word) - ACRONYMS.include?(word.to_s.upcase) - end - - extend self end end diff --git a/spec/killbill_client/resource_spec.rb b/spec/killbill_client/resource_spec.rb index 66404ac..315b046 100644 --- a/spec/killbill_client/resource_spec.rb +++ b/spec/killbill_client/resource_spec.rb @@ -77,4 +77,39 @@ def require_multi_tenant_options! end end end + + describe '.from_json' do + let(:json) do + { + 'accountId' => 'abcd', + 'accountCBA' => 12.5, + 'externalKey' => 'ext', + 'unknownAttribute' => 'ignored', + 'auditLogs' => [{ 'changeType' => 'INSERT', 'changeDate' => '2024-01-01' }] + }.to_json + end + + it 'maps camelCased JSON keys onto attributes, including nested resources and acronyms' do + # Twice, to exercise both the cold and the cached paths + 2.times do + account = described_class.from_json(KillBillClient::Model::Account, json) + + expect(account.account_id).to eq('abcd') + expect(account.account_cba).to eq(12.5) + expect(account.external_key).to eq('ext') + expect(account.audit_logs.size).to eq(1) + expect(account.audit_logs.first).to be_a(KillBillClient::Model::AuditLog) + expect(account.audit_logs.first.change_type).to eq('INSERT') + end + end + + it 'round-trips through to_hash' do + account = described_class.from_json(KillBillClient::Model::Account, json) + hash = account.to_hash + + expect(hash['accountId']).to eq('abcd') + expect(hash['accountCBA']).to eq(12.5) + expect(KillBillClient::Model::Account.new(hash)).to eq(account) + end + end end diff --git a/spec/killbill_client/utils_spec.rb b/spec/killbill_client/utils_spec.rb new file mode 100644 index 0000000..cb0605d --- /dev/null +++ b/spec/killbill_client/utils_spec.rb @@ -0,0 +1,140 @@ +require 'spec_helper' + +describe KillBillClient::Utils do + describe '.underscore' do + { + 'accountId' => 'account_id', + 'accountCBA' => 'account_cba', + 'CBAAmount' => 'cba_amount', + 'HTTPResponseCode' => 'http_response_code', + 'invoice2Id' => 'invoice2_id', + 'a1B' => 'a1_b', + 'KillBillClient::Model::Account' => 'kill_bill_client/model/account', + 'billing-period' => 'billing_period', + 'Billing-Period' => 'billing_period', + 'ÉtatCivil' => 'état_civil', + 'already_underscored' => 'already_underscored', + 'lowercase' => 'lowercase', + '' => '' + }.each do |input, expected| + it "converts #{input.inspect} to #{expected.inspect}" do + expect(described_class.underscore(input)).to eq(expected) + end + end + + it 'accepts symbols and other objects responding to to_s' do + expect(described_class.underscore(:accountId)).to eq('account_id') + expect(described_class.underscore(nil)).to eq('') + expect(described_class.underscore(42)).to eq('42') + end + + it 'is idempotent' do + %w(accountId accountCBA CBAAmount a1B billing-period KillBillClient::Model::Account).each do |input| + once = described_class.underscore(input) + expect(described_class.underscore(once)).to eq(once) + end + end + + it 'returns frozen strings, and caches them' do + first = described_class.underscore('someFreshCamelCasedName') + expect(first).to be_frozen + expect(described_class.underscore('someFreshCamelCasedName')).to equal(first) + expect(described_class.underscore('already_underscored_name')).to be_frozen + end + + it 'does not alias or mutate its input' do + input = +'accountId' + result = described_class.underscore(input) + input << 'Suffix' + expect(result).to eq('account_id') + expect(described_class.underscore('accountId')).to eq('account_id') + expect(described_class.underscore(input)).to eq('account_id_suffix') + end + + it 'is thread safe' do + names = (0...200).map { |i| "someAttributeName#{i}Value" } + results = 8.times.map do + Thread.new { names.map { |name| described_class.underscore(name) } } + end.map(&:value) + + expected = names.map { |name| name.gsub(/([a-z\d])([A-Z])/, '\1_\2').downcase } + results.each { |result| expect(result).to eq(expected) } + end + end + + describe '.underscore_symbols' do + it 'returns the attribute name and its setter' do + expect(described_class.underscore_symbols('accountCBA')).to eq([:account_cba, :account_cba=]) + expect(described_class.underscore_symbols(:accountId)).to eq([:account_id, :account_id=]) + expect(described_class.underscore_symbols('account_id')).to eq([:account_id, :account_id=]) + end + + it 'returns frozen, cached results' do + first = described_class.underscore_symbols('anotherFreshName') + expect(first).to be_frozen + expect(described_class.underscore_symbols('anotherFreshName')).to equal(first) + end + end + + describe '.camelize' do + { + ['account_id'] => 'AccountId', + ['account_id', :lower] => 'accountId', + ['account_cba'] => 'AccountCBA', + ['account_cba', :lower] => 'accountCBA', + ['cba'] => 'CBA', + # Legacy behavior, unchanged: the acronym is upcased before the first letter is lowered + ['cba', :lower] => 'cBA', + ['cba_amount', :lower] => 'cBAAmount', + ['account'] => 'Account', + ['account', :lower] => 'account', + ['invoice2_id', :lower] => 'invoice2Id', + ['accountId'] => 'AccountId', + ['accountId', :lower] => 'accountId', + [''] => '' + }.each do |args, expected| + it "converts #{args.inspect} to #{expected.inspect}" do + expect(described_class.camelize(*args)).to eq(expected) + end + end + + it 'accepts symbols' do + expect(described_class.camelize(:account_id, :lower)).to eq('accountId') + end + + it 'caches upper and lower variants independently' do + expect(described_class.camelize('some_name')).to eq('SomeName') + expect(described_class.camelize('some_name', :lower)).to eq('someName') + expect(described_class.camelize('some_name')).to eq('SomeName') + end + + it 'returns frozen strings, and caches them' do + first = described_class.camelize('some_fresh_name', :lower) + expect(first).to be_frozen + expect(described_class.camelize('some_fresh_name', :lower)).to equal(first) + end + + it 'does not cache failures' do + expect { described_class.camelize('', :lower) }.to raise_error(TypeError) + expect { described_class.camelize('', :lower) }.to raise_error(TypeError) + end + end + + describe KillBillClient::Utils::Cache do + it 'stops caching once full, but keeps computing correct values' do + cache = described_class.new(2) + calls = 0 + compute = lambda { |value| cache.fetch(value) { calls += 1; value.to_s * 2 } } + + expect(compute.call('a')).to eq('aa') + expect(compute.call('b')).to eq('bb') + expect(compute.call('c')).to eq('cc') + expect(cache.size).to eq(2) + expect(calls).to eq(3) + + expect(compute.call('a')).to eq('aa') + expect(compute.call('c')).to eq('cc') + expect(calls).to eq(4) + end + end +end