Skip to content

Optimize _is_collection? method #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 28, 2025

Conversation

moberegger
Copy link

Saw the _is_collection? helper method showing up as a hotspot in profiles for some of our larger templates.

Running the .all? is 3.75x slower than simply doing a couple of respond_to? comparisons. Comparing the two _is_collection? method implementations directly:

ruby 3.4.2 (2025-02-15 revision d2930f8e7a) +YJIT +PRISM [arm64-darwin24]
Warming up --------------------------------------
                 new     2.287M i/100ms
                 old   875.646k i/100ms
Calculating -------------------------------------
                 new     39.004M (± 1.3%) i/s   (25.64 ns/i) -    196.665M in   5.043030s
                 old     10.398M (± 3.9%) i/s   (96.18 ns/i) -     52.539M in   5.062959s

Comparison:
                 new: 39003805.0 i/s
                 old: 10397648.9 i/s - 3.75x  slower

As a side bonus, this also prevents an additional array allocation from when _object_respond_to? splats the provided method symbols, which adds up for larger responses that use multiple partials.

Calculating -------------------------------------
                 new     0.000  memsize (     0.000  retained)
                         0.000  objects (     0.000  retained)
                         0.000  strings (     0.000  retained)
                 old    40.000  memsize (     0.000  retained)
                         1.000  objects (     0.000  retained)
                         0.000  strings (     0.000  retained)

Comparison:
                 new:          0 allocated
                 old:         40 allocated - Infx more

A simple benchmark to compare results through the jbuilder API itself. This is the most minimalistic way I found to exercise the condition.

some_array = Array.new(10) { |i| { id: i, name: "name_#{i}" } }
json = Jbuilder.new

Benchmark.ips do |x|
  x.report('old') do
    json.set! :foo, some_array, :id, :name
  end

  x.report('new') do
    json.set! :foo, some_array, :id, :name
  end

  x.hold! 'temp_results'
  x.compare!
end
ruby 3.4.2 (2025-02-15 revision d2930f8e7a) +YJIT +PRISM [arm64-darwin24]
Warming up --------------------------------------
                 old    18.514k i/100ms
                 new    19.520k i/100ms
Calculating -------------------------------------
                 old    189.515k (± 1.8%) i/s    (5.28 μs/i) -    962.728k in   5.081742s
                 new    198.868k (± 2.5%) i/s    (5.03 μs/i) -    995.520k in   5.009296s

Comparison:
                 new:   198867.8 i/s
                 old:   189515.1 i/s - 1.05x  slower

So ~5% faster, although performance characteristics will depend on how often your template exercises the condition.

Already filed upstream in rails#590

@moberegger moberegger requested review from mscrivo and Insomniak47 May 28, 2025 18:30
Comment on lines -366 to -368
def _object_respond_to?(object, *methods)
methods.all?{ |m| object.respond_to?(m) }
end
Copy link
Author

Choose a reason for hiding this comment

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

As an aside, you see a lot of this sort of arg splatting throughout jbuilder, which really adds up in terms of memory allocations. Some are necessary to support things like json.foo @foo, :id, :name, :etc, but others like this can be optimized away.

@moberegger moberegger merged commit a1ddb07 into main May 28, 2025
30 checks passed
@moberegger moberegger deleted the moberegger/optimize_is_collection branch June 13, 2025 18:54
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.

2 participants