Improve read_multi_entries performance for numerous keys

[vinistock]

Summary

This suggestion is a minor refactor of ActiveSupport::Cache::Store#read_multi_entries. It changes three aspects:

1. Use each_with_object instead of each so that we don't have to declare the hash in the first line and return it in the last one
2. Avoid computing the local variable version if we don't find a cache entry
3. Remove no op conditional path

This patch improves performance slightly as the number of keys fetched increases and - in my opinion - makes the method a little bit more readable. The benchmark script can be found below.

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem "rails", github: "rails/rails"
  gem "benchmark-ips"
end

require "active_support"

module ActiveSupport
  module Cache
    class Store
      def fast_read_multi(*names)
        options = names.extract_options!
        options = merged_options(options)

        instrument :read_multi, names, options do |payload|
          fast_read_multi_entries(names, **options).tap do |results|
            payload[:hits] = results.keys
          end
        end
      end

      private
        def fast_read_multi_entries(names, **options)
          names.each_with_object({}) do |name, results|
            key   = normalize_key(name, options)
            entry = read_entry(key, **options)

            next unless entry

            version = normalize_version(name, options)

            if entry.expired?
              delete_entry(key, **options)
            elsif !entry.mismatched?(version)
              results[name] = entry.value
            end
          end
        end
    end
  end
end

# Enumerate some representative scenarios here.
#
# It is very easy to make an optimization that improves performance for a
# specific scenario you care about but regresses on other common cases.
# Therefore, you should test your change against a list of representative
# scenarios. Ideally, they should be based on real-world scenarios extracted
# from production applications.
SCENARIOS = {
  "A few keys" => %w[key_0 key_1 key_2],
  "Many keys" => (0..1_000).map { |i| "key_#{i}" },
}

SCENARIOS.each_pair do |name, value|
  puts
  puts " #{name} ".center(80, "=")
  puts

  cache = ActiveSupport::Cache::MemoryStore.new

  (0..1_000).each do |i|
    cache.write("key_#{i}", "value_#{i}")
  end

  Benchmark.ips do |x|
    x.report("read_multi")      { cache.read_multi(value) }
    x.report("fast_read_multi") { cache.fast_read_multi(value) }
    x.compare!
  end
end


================================== A few keys ==================================

Warming up --------------------------------------
          read_multi    12.831k i/100ms
     fast_read_multi    14.510k i/100ms
Calculating -------------------------------------
          read_multi    146.288k (±26.0%) i/s -    654.381k in   5.010593s
     fast_read_multi    172.428k (±25.9%) i/s -    783.540k in   5.023852s

Comparison:
     fast_read_multi:   172427.5 i/s
          read_multi:   146288.2 i/s - same-ish: difference falls within error


================================== Many keys ===================================

Warming up --------------------------------------
          read_multi   196.000  i/100ms
     fast_read_multi   279.000  i/100ms
Calculating -------------------------------------
          read_multi      1.984k (± 6.7%) i/s -      9.996k in   5.062818s
     fast_read_multi      2.823k (± 7.6%) i/s -     14.229k in   5.072824s

Comparison:
     fast_read_multi:     2823.1 i/s
          read_multi:     1984.3 i/s - 1.42x  slower