Skip to content

Commit

Permalink
[#3484] Add missing argument to Hash default proc (#3486)
Browse files Browse the repository at this point in the history
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 <christinach@users.noreply.github.com>
Co-authored-by: Kevin Reiss <kevinreiss@users.noreply.github.com>
Co-authored-by: Max Kadel <maxkadel@users.noreply.github.com>
  • Loading branch information
4 people authored Jan 7, 2025
1 parent 8afe948 commit 03ea265
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 1 deletion.
1 change: 1 addition & 0 deletions blacklight.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ Gem::Specification.new do |s|
s.add_development_dependency "rubocop-capybara"
s.add_development_dependency "rubocop-rspec_rails"
s.add_development_dependency "rubocop-factory_bot"
s.add_development_dependency "rspec-benchmark"
s.add_development_dependency "i18n-tasks", '~> 1.0'
s.add_development_dependency "solr_wrapper"
end
2 changes: 1 addition & 1 deletion lib/blacklight/solr/response/facets.rb
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ def merge_facet(name:, value:, hits: nil)
# facet data in the response
def default_aggregations
@default_aggregations ||= begin
h = Hash.new { |key| null_facet_field_object(key) }
h = Hash.new { |_hash, key| null_facet_field_object(key) }
h.with_indifferent_access
end
end
Expand Down
Loading

0 comments on commit 03ea265

Please sign in to comment.