[backport 8.x] Add missing argument to Hash default proc #3486
Conversation
This file contains 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
The proc to fill in a default value for a Hash accepts two arguments: the hash itself and then the key. Prior to this commit, our proc was only using the Hash, but was treating it as if it was a key. This had the potential to cause some serious performance problems when solr returned many facets or many facet values, when a huge hash was treated as a string and interpolated into other strings via Hash#inspect. Adding this missing argument ensures that we send the key/field_name, rather than the entire hash. For the provided test case on my laptop, this commit speed up null facet object generation up from 45 iterations/second to 18,700 iterations/second. It also doubles the performance of our advanced search form. This commit also adds the rspec-benchmark gem as a dev dependency -- I'm definitely open to figuring out a different way to test this if the additional dependency is not desired. Closes #3484 Co-authored-by: Christina Chortaria <[email protected]> Co-authored-by: Kevin Reiss <[email protected]> Co-authored-by: Max Kadel <[email protected]>
bess
approved these changes
Jan 7, 2025
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.
This is a backport of #3485