diff --git a/benchmark/conversions.rb b/benchmark/conversions.rb new file mode 100644 index 0000000..8ed2197 --- /dev/null +++ b/benchmark/conversions.rb @@ -0,0 +1,69 @@ +#!/usr/bin/env ruby +# +# Micro-benchmark for the hot path that turns Kill Bill JSON responses into models (and back): +# Utils.underscore / Utils.camelize and Resource.from_json / #to_hash. +# +# Only uses the standard library. Run it on two checkouts (e.g. master vs a branch) to compare: +# +# ruby -Ilib benchmark/conversions.rb +# jruby -Ilib benchmark/conversions.rb +# +# ITERATIONS (default 20) and INVOICES (default 300, i.e. a large account) can be set in the environment. +# On CRuby the number of allocated objects per operation is reported too. + +require 'json' +require 'killbill_client' + +ITERATIONS = Integer(ENV['ITERATIONS'] || 20) +INVOICES = Integer(ENV['INVOICES'] || 300) +ITEMS_PER_INVOICE = 3 +WARMUP_ROUNDS = 5 + +def json_hash_for(model_class, overrides = {}) + model_class.json_attributes.each_with_object({}) do |name, hash| + hash[KillBillClient::Utils.camelize(name, :lower)] = "value-#{name}" + end.merge(overrides) +end + +# A list of invoices with items, shaped like GET /1.0/kb/accounts/{id}/invoices?withItems=true +invoices = Array.new(INVOICES) do |i| + items = Array.new(ITEMS_PER_INVOICE) { |j| json_hash_for(KillBillClient::Model::InvoiceItem, 'amount' => j * 10.0) } + json_hash_for(KillBillClient::Model::Invoice, 'amount' => i * 1.5, 'items' => items, 'credits' => [], 'auditLogs' => []) +end +INVOICES_JSON = JSON.generate(invoices) +KEYS = (KillBillClient::Model::Invoice.json_attributes + KillBillClient::Model::InvoiceItem.json_attributes + + KillBillClient::Model::Account.json_attributes).uniq.map { |n| KillBillClient::Utils.camelize(n, :lower) } + +def allocated_objects + GC.respond_to?(:stat) && GC.stat.key?(:total_allocated_objects) ? GC.stat(:total_allocated_objects) : nil +end + +def measure(label, ops_per_iteration) + WARMUP_ROUNDS.times { yield } + before_allocs = allocated_objects + started = Process.clock_gettime(Process::CLOCK_MONOTONIC) + ITERATIONS.times { yield } + elapsed = Process.clock_gettime(Process::CLOCK_MONOTONIC) - started + after_allocs = allocated_objects + + ops = ITERATIONS * ops_per_iteration + line = format('%-42s %10.1f ms/iter %12.0f ops/s', label, elapsed * 1000 / ITERATIONS, ops / elapsed) + line += format(' %10.1f objs/op', (after_allocs - before_allocs).to_f / ops) if before_allocs && after_allocs + puts line +end + +puts "#{RUBY_ENGINE} #{RUBY_ENGINE == 'jruby' ? JRUBY_VERSION : RUBY_VERSION}, killbill-client #{KillBillClient::VERSION}, " \ + "#{INVOICES} invoices x #{ITEMS_PER_INVOICE} items (#{INVOICES_JSON.bytesize / 1024} KiB of JSON), #{ITERATIONS} iterations" + +measure("Utils.underscore (#{KEYS.size} keys x 1000)", KEYS.size * 1000) do + 1000.times { KEYS.each { |k| KillBillClient::Utils.underscore(k) } } +end + +records = nil +measure('Resource.from_json (invoice list)', INVOICES) do + records = KillBillClient::Model::Resource.send(:from_json, KillBillClient::Model::Invoice, INVOICES_JSON) +end + +measure('Resource#to_hash (invoice list)', INVOICES) do + records.each(&:to_hash) +end diff --git a/lib/killbill_client/models/resource.rb b/lib/killbill_client/models/resource.rb index c752f20..d161cbb 100644 --- a/lib/killbill_client/models/resource.rb +++ b/lib/killbill_client/models/resource.rb @@ -172,7 +172,8 @@ 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 + # name was already underscored at the top of the loop + record.send("#{name}=", value) rescue nil end #end data.each diff --git a/lib/killbill_client/utils.rb b/lib/killbill_client/utils.rb index 88c600d..3116aa1 100644 --- a/lib/killbill_client/utils.rb +++ b/lib/killbill_client/utils.rb @@ -2,7 +2,65 @@ module KillBillClient module Utils ACRONYMS = %w(CBA).freeze + # Upper bound on the number of cached conversions per cache. Attribute names come from the Kill Bill API, + # so the set of distinct inputs is small in practice: this is only a safety net against unbounded growth. + MAX_CACHE_SIZE = 10_000 + + # Thread-safe memoization for the string conversions below. They run for every JSON key of every + # Kill Bill response and request, on a small set of distinct inputs, so caching them avoids a lot of + # regex work and allocations (in particular on JRuby). + # + # Reads never take the lock: they read a frozen Hash, and writers replace it with an updated frozen copy. + # Cached values are frozen, since they are shared between callers. + class ConversionCache + def initialize(max_size = MAX_CACHE_SIZE) + @max_size = max_size + @entries = {}.freeze + @lock = Mutex.new + end + + def fetch(key) + value = @entries[key] + return value unless value.nil? + + value = yield(key).freeze + @lock.synchronize do + if @entries.size < @max_size && !@entries.key?(key) + @entries = @entries.merge((key.frozen? ? key : key.dup.freeze) => value).freeze + end + end + value + end + + def size + @entries.size + end + end + + UNDERSCORE_CACHE = ConversionCache.new + CAMELIZE_UPPER_CACHE = ConversionCache.new + CAMELIZE_LOWER_CACHE = ConversionCache.new + + # Note: the returned String is frozen (and shared), dup it before modifying it. def camelize(underscored_word, first_letter = :upper) + cache = first_letter == :lower ? CAMELIZE_LOWER_CACHE : CAMELIZE_UPPER_CACHE + cache.fetch(underscored_word.to_s) { |word| camelize_uncached(word, first_letter) } + end + + def demodulize(class_name_in_module) + class_name_in_module.to_s.sub(/^.*::/, '') + end + + # Note: the returned String is frozen (and shared), dup it before modifying it. + def underscore(camel_cased_word) + UNDERSCORE_CACHE.fetch(camel_cased_word.to_s) { |word| underscore_uncached(word) } + end + + def acronym?(word) + ACRONYMS.include?(word.to_s.upcase) + end + + def camelize_uncached(underscored_word, first_letter = :upper) camelized = underscored_word.to_s.split('_').map do |word| if acronym?(word) word.upcase @@ -14,24 +72,18 @@ 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 underscore_uncached(camel_cased_word) word = camel_cased_word.to_s.dup word.gsub!(/::/, '/') - word.gsub!(/([A-Z]+)([A-Z][a-z])/, '\1_\2') + # Same result as /([A-Z]+)([A-Z][a-z])/ => '\1_\2' ("HTTPResponse" => "HTTP_Response"), but without the + # quantifier that backtracks polynomially on long runs of capitals (36s for 50,000 "A"s on JRuby). + word.gsub!(/([A-Z])(?=[A-Z][a-z])/, '\1_') word.gsub!(/([a-z\d])([A-Z])/, '\1_\2') word.tr! '-', '_' word.downcase! word end - def acronym?(word) - ACRONYMS.include?(word.to_s.upcase) - end - extend self end end diff --git a/spec/killbill_client/utils_spec.rb b/spec/killbill_client/utils_spec.rb new file mode 100644 index 0000000..b05647f --- /dev/null +++ b/spec/killbill_client/utils_spec.rb @@ -0,0 +1,181 @@ +require 'spec_helper' + +describe KillBillClient::Utils do + # Real attribute names, as sent by Kill Bill (camelCase) and as used by the models (snake_case) + let(:camel_cased) do + %w(accountId externalKey billCycleDayLocal accountCBA accountBalance isPaymentDelegatedToParent + paymentMethodId referenceTime timeZone CBA HTTPResponse X-Killbill-ApiKey Foo::BarBaz already_underscored id) + end + let(:underscored) do + %w(account_id external_key bill_cycle_day_local account_cba account_balance + is_payment_delegated_to_parent payment_method_id reference_time time_zone cba id) + end + + describe '.underscore' do + { + 'accountId' => 'account_id', + 'accountCBA' => 'account_cba', + 'CBA' => 'cba', + 'HTTPResponse' => 'http_response', + 'X-Killbill-ApiKey' => 'x_killbill_api_key', + 'Foo::BarBaz' => 'foo/bar_baz', + 'already_underscored' => 'already_underscored', + 'id' => 'id', + '' => '' + }.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 nil, like before' do + expect(described_class.underscore(:accountId)).to eq('account_id') + expect(described_class.underscore(nil)).to eq('') + end + + it 'returns the same result as the uncached implementation' do + camel_cased.each do |word| + expect(described_class.underscore(word)).to eq(described_class.underscore_uncached(word)) + end + end + + # The implementation from before the caching change, which used /([A-Z]+)([A-Z][a-z])/ + def original_underscore(camel_cased_word) + word = camel_cased_word.to_s.dup + word.gsub!(/::/, '/') + word.gsub!(/([A-Z]+)([A-Z][a-z])/, '\1_\2') + word.gsub!(/([a-z\d])([A-Z])/, '\1_\2') + word.tr! '-', '_' + word.downcase! + word + end + + it 'matches the original implementation on tricky and random inputs' do + tricky = %w(HTTPResponse XMLHttpRequest ABCdEFg ABcDEf AbCDe AAAb aAAb A Aa AA AAa getHTTPStatusCode + account2FA x-Killbill-ApiKey Foo::BARBaz) + tricky.each { |word| expect(described_class.underscore_uncached(word)).to eq(original_underscore(word)) } + + alphabet = ('A'..'Z').to_a + ('a'..'z').to_a + %w(0 1 _ - :) + rng = Random.new(42) + 2_000.times do + word = Array.new(rng.rand(1..14)) { alphabet[rng.rand(alphabet.size)] }.join + expect(described_class.underscore_uncached(word)).to eq(original_underscore(word)) + end + end + + it 'runs in linear time on long runs of capital letters' do + started = Process.clock_gettime(Process::CLOCK_MONOTONIC) + expect(described_class.underscore_uncached('A' * 50_000)).to eq('a' * 50_000) + expect(Process.clock_gettime(Process::CLOCK_MONOTONIC) - started).to be < 2 + end + + it 'is idempotent, so underscoring an already underscored name is a no-op' do + camel_cased.each do |word| + once = described_class.underscore(word) + expect(described_class.underscore(once)).to eq(once) + end + end + + it 'returns a frozen, cached String' do + first = described_class.underscore('paymentMethodId') + expect(first).to be_frozen + expect(described_class.underscore('paymentMethodId')).to be(first) + end + + it 'is not affected by later changes to the input String' do + input = +'subscriptionId' + expect(described_class.underscore(input)).to eq('subscription_id') + input << 'Suffix' + expect(described_class.underscore('subscriptionId')).to eq('subscription_id') + expect(described_class.underscore(input)).to eq('subscription_id_suffix') + end + end + + describe '.camelize' do + { + ['account_id', :upper] => 'AccountId', + ['account_id', :lower] => 'accountId', + ['account_cba', :lower] => 'accountCBA', + ['cba', :upper] => 'CBA', + ['bill_cycle_day_local', :lower] => 'billCycleDayLocal', + ['id', :lower] => 'id' + }.each do |(input, first_letter), expected| + it "converts #{input.inspect} (#{first_letter}) to #{expected.inspect}" do + expect(described_class.camelize(input, first_letter)).to eq(expected) + end + end + + it 'defaults to an upper case first letter' do + expect(described_class.camelize('account_id')).to eq('AccountId') + end + + it 'caches upper and lower case variants separately' do + expect(described_class.camelize('tenant_id', :lower)).to eq('tenantId') + expect(described_class.camelize('tenant_id', :upper)).to eq('TenantId') + expect(described_class.camelize('tenant_id', :lower)).to eq('tenantId') + end + + it 'returns the same result as the uncached implementation' do + underscored.each do |word| + [:upper, :lower].each do |first_letter| + expect(described_class.camelize(word, first_letter)).to eq(described_class.camelize_uncached(word, first_letter)) + end + end + end + + it 'returns a frozen, cached String' do + first = described_class.camelize('payment_id', :lower) + expect(first).to be_frozen + expect(described_class.camelize('payment_id', :lower)).to be(first) + end + end + + describe KillBillClient::Utils::ConversionCache do + it 'computes a value once per key' do + cache = described_class.new + calls = 0 + 2.times { expect(cache.fetch('a') { |k| calls += 1; k.upcase }).to eq('A') } + expect(calls).to eq(1) + expect(cache.size).to eq(1) + end + + it 'stops caching once full, but keeps returning correct values' do + cache = described_class.new(2) + %w(a b c d).each { |k| expect(cache.fetch(k, &:upcase)).to eq(k.upcase) } + expect(cache.size).to eq(2) + expect(cache.fetch('d', &:upcase)).to eq('D') + end + + it 'is safe to use from many threads at once' do + cache = described_class.new + words = (1..200).map { |i| "attributeName#{i}" } + results = Array.new(8) do + Thread.new { words.shuffle.map { |w| [w, cache.fetch(w) { |k| KillBillClient::Utils.underscore_uncached(k) }] } } + end.map(&:value) + + results.each do |pairs| + pairs.each { |w, v| expect(v).to eq(KillBillClient::Utils.underscore_uncached(w)) } + end + expect(cache.size).to eq(words.size) + end + end + + describe 'JSON round trip through the models' do + it 'maps camelCase JSON keys to model attributes and back' do + json = { 'accountId' => 'a1', 'externalKey' => 'ext', 'accountCBA' => 12.5, 'billCycleDayLocal' => 3, + 'unknownAttribute' => 'ignored' }.to_json + + account = KillBillClient::Model::Account.send(:from_json, KillBillClient::Model::Account, json) + expect(account.account_id).to eq('a1') + expect(account.external_key).to eq('ext') + expect(account.account_cba).to eq(12.5) + expect(account.bill_cycle_day_local).to eq(3) + + hash = account.to_hash + expect(hash['accountId']).to eq('a1') + expect(hash['accountCBA']).to eq(12.5) + expect(hash['billCycleDayLocal']).to eq(3) + expect(hash).not_to have_key('unknownAttribute') + end + end +end