Skip to content

Conversation

@moberegger
Copy link

Filed upstream: rails#607

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-blocking path for the most likely case.

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 hottest 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 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

@moberegger moberegger requested review from a team and Insomniak47 December 5, 2025 20:03
Copy link

@Insomniak47 Insomniak47 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvement (and application of something you literally just saw :P)

@moberegger moberegger merged commit 998b3aa into main Dec 5, 2025
30 checks passed
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.

3 participants