Optimize _is_collection? method#2
Merged
Merged
Conversation
moberegger
commented
May 28, 2025
Comment on lines
-366
to
-368
| def _object_respond_to?(object, *methods) | ||
| methods.all?{ |m| object.respond_to?(m) } | ||
| end |
Author
There was a problem hiding this comment.
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.
mscrivo
approved these changes
May 28, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 ofrespond_to?comparisons. Comparing the two_is_collection?method implementations directly: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.A simple benchmark to compare results through the jbuilder API itself. This is the most minimalistic way I found to exercise the condition.
So ~5% faster, although performance characteristics will depend on how often your template exercises the condition.
Already filed upstream in rails#590