Skip to content

Conversation

@moberegger
Copy link
Contributor

@moberegger moberegger commented Dec 5, 2025

Builds on top of #597. This PR optimizes the KeyFormatter so that it doesn't lock the mutex on a cache hit by implementing the check-lock-check pattern, which gives us a fast non-locking path for the most likely case. This was inspired by how Concurrent::Map is implemented.

The mutex will only be locked when there is a cache miss, and while locked it will first attempt another read to account for the situation where another thread has already computed the value.

This small change results in a significant improvement on cache hits when running Jbuilder::KeyFormatter#format.

ruby 3.4.7 (2025-10-08 revision 7a5688e2a2) +YJIT +PRISM [arm64-darwin24]
Warming up --------------------------------------
              before   852.760k i/100ms
               after     1.566M i/100ms
Calculating -------------------------------------
              before     10.275M (± 4.7%) i/s   (97.32 ns/i) -     52.018M in   5.077786s
               after     23.927M (± 1.7%) i/s   (41.79 ns/i) -    120.613M in   5.042395s

Comparison:
              before: 10274977.5 i/s
               after: 23926727.6 i/s - 2.33x  faster

When using key formatting, this is one of the hotter code paths in jbuilder. The following benchmark

json = Jbuilder.new(key_formatter: Jbuilder::KeyFormatter.new(:downcase))

Benchmark.ips do |x|
  x.report('before') do |n|
    n.times { json.set! :Foo, :bar }
  end
  x.report('after') do |n|
    n.times { json.set! :Foo, :bar }
  end

  x.hold! 'temp_ips'
  x.compare!(order: :baseline)
end

Shows a 1.29x improvement

ruby 3.4.7 (2025-10-08 revision 7a5688e2a2) +YJIT +PRISM [arm64-darwin24]
Warming up --------------------------------------
              before   416.819k i/100ms
               after   521.578k i/100ms
Calculating -------------------------------------
              before      4.641M (± 2.9%) i/s  (215.45 ns/i) -     23.342M in   5.034002s
               after      5.993M (± 3.3%) i/s  (166.86 ns/i) -     30.252M in   5.054337s

Comparison:
              before:  4641344.3 i/s
               after:  5993038.9 i/s - 1.29x  faster

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.

1 participant