Cache Utils.underscore and Utils.camelize (3x faster from_json on JRuby)

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

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
 

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

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