Use transform_values to build hash_rows

[jhawthorn]

This commit changes hash_rows to build the first row as it did before, and then to use transform_values to build subsequent rows, using the first as a template.

In Ruby 2.4+ (first version to include transform_values) this is a tiny bit faster because the hash will never be realloc'd and the hash update logic is probably slightly simpler than add.

In Ruby 2.7+ this is a fair bit faster because transform_values is able to run without hashing the keys and only iterates over the values list.

In the case that we only have one row this is only one extra lvar set/read and the condition.

Benchmark

# frozen_string_literal: true
require "active_support/all"

columns = %w[id author_id section_id title byline body published_at created_at updated_at].map(&:-@)

row = [1, 2, 3, "Top 10 tricks to speed up hashes", "by John Hawthorn", "transform_values is quick and even faster in Ruby 2.7. Try it today!", Time.now, Time.now, Time.now]
rows = [row] * ENV.fetch('N', 10).to_i

def hash_like_rails_6_0(columns, row)
  length = columns.length
  hash = {}

  index = 0
  while index < length
    hash[columns[index]] = row[index]
    index += 1
  end

  hash
end

Benchmark.ips do |x|
  x.report "Rails 6.0" do
    rows.map { |row| hash_like_rails_6_0(columns, row) }
  end

  x.report "transform_values" do
    template = nil
    rows.map do |row|
      if template
        index = -1
        template.transform_values do
          row[index += 1]
        end
      else
        template = hash_like_rails_6_0(columns, row)
      end
    end
  end
end

I made a graph to show the current Rails 6.0 implementation vs transform_values on Ruby 2.6 and 2.7. Interestingly, the current Rails 6.0 implementation is actually slower on Ruby 2.7 (a regression I'm looking into), but that shouldn't affect the motivation for this change.

This is about 7% faster on Ruby 2.6 and Ruby 2.7+transform_values is 25% faster than Ruby 2.6+current implementation (using this measurement because Ruby 2.7's hash perf regression)

[kaspth]

row = [1, 2, 3, "Top 10 tricks to speed up hashes", "by John Hawthorn", "transform_values is quick and even faster in Ruby 2.7. Try it today!", Time.now, Time.now, Time.now]

lol! Very much approved on this, though I'm not seeing where template is assigned to yet. I'm guessing that's coming soon.

[jhawthorn]

I'm not seeing where template is assigned to yet

🤦‍♂ fixed

[kaspth]

Sweet!

[p8]

Should this indentation be fixed?

[jhawthorn]

Tracked the 2.7 performance difference down to this commit ruby/ruby@72825c3 so the performance difference is intentional and we're just hitting the case where it's slower (hash construction) and my 9-column example happens to be the worst case 😅. In most cases that optimization is probably well worth it.

[rails-bot]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

[kaspth]

@vinistock since you've been doing some perf work, curious if you want to test if either of these are improvements speed/allocation wise:

hash = {}
0.upto length do |index|
  hash[columns[index]] = row[index]
end

hash = {}
columns.each_with_index do |column, index|
  hash[column] = row[index]
end

[vinistock]

Absolutely! I benchmarked the current implementation vs the two options you suggested. For this benchmark, I removed the memoization using the instance variable.

It appears that the current implementation performs better than the others. And memory wise, it looks like they all perform equally.

Script

# 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", require: "rails/all"
  gem "benchmark-ips"
  gem "benchmark-memory", require: "benchmark/memory"
end

module ActiveRecord
  class Result
    private

    def hash_rows
      columns = @columns.map(&:-@)
      length  = columns.length
      template = nil

      @rows.map { |row|
        if template
          index = -1
          template.transform_values do
            row[index += 1]
          end
        else
          hash = {}

          index = 0
          while index < length
            hash[columns[index]] = row[index]
            index += 1
          end

          template = hash if hash.length == length
          hash
        end
      }
    end

    def upto_hash_rows
      columns = @columns.map(&:-@)
      length  = columns.length
      template = nil

      @rows.map { |row|
        if template
          index = -1
          template.transform_values do
            row[index += 1]
          end
        else
          hash = {}
          0.upto length do |index|
            hash[columns[index]] = row[index]
          end

          template = hash if hash.length == length
          hash
        end
      }
    end

    def each_hash_rows
      columns = @columns.map(&:-@)
      length  = columns.length
      template = nil

      @rows.map { |row|
        if template
          index = -1
          template.transform_values do
            row[index += 1]
          end
        else
          hash = {}
          columns.each_with_index do |column, index|
            hash[column] = row[index]
          end

          template = hash if hash.length == length
          hash
        end
      }
    end
  end
end

result = ActiveRecord::Result.new((0..50).map { |i| "col_#{i}" }, (0..1000).map { |row| (0..50).map { |col| "row_#{row}_col_#{col}" } })

Benchmark.ips do |x|
  x.report("hash_rows")      { result.send(:hash_rows) }
  x.report("upto_hash_rows") { result.send(:upto_hash_rows) }
  x.report("each_hash_rows") { result.send(:each_hash_rows) }
  x.compare!
end

Benchmark.memory do |x|
  x.report("hash_rows")      { result.send(:hash_rows) }
  x.report("upto_hash_rows") { result.send(:upto_hash_rows) }
  x.report("each_hash_rows") { result.send(:each_hash_rows) }
  x.compare!
end

Results

Warming up --------------------------------------
           hash_rows    18.000  i/100ms
      upto_hash_rows    13.000  i/100ms
      each_hash_rows    25.000  i/100ms
Calculating -------------------------------------
           hash_rows    277.793  (±20.5%) i/s -      1.242k in   5.011782s
      upto_hash_rows    148.981  (±14.8%) i/s -    715.000  in   5.066219s
      each_hash_rows    277.071  (±20.2%) i/s -      1.275k in   5.059477s

Comparison:
           hash_rows:      277.8 i/s
      each_hash_rows:      277.1 i/s - same-ish: difference falls within error
      upto_hash_rows:      149.0 i/s - 1.86x  slower

Calculating -------------------------------------
           hash_rows     1.772M memsize (     0.000  retained)
                         1.054k objects (     0.000  retained)
                        50.000  strings (     0.000  retained)
      upto_hash_rows     1.772M memsize (     0.000  retained)
                         1.054k objects (     0.000  retained)
                        50.000  strings (     0.000  retained)
      each_hash_rows     1.772M memsize (     0.000  retained)
                         1.054k objects (     0.000  retained)
                        50.000  strings (     0.000  retained)

Comparison:
           hash_rows:    1772296 allocated
      upto_hash_rows:    1772296 allocated - same
      each_hash_rows:    1772296 allocated - same

[kaspth]

Awesome, thanks!

           hash_rows:      277.8 i/s
      each_hash_rows:      277.1 i/s - same-ish: difference falls within error

Seems like the each_with_index version is just as fast? I do like that we'd remove the index local variable reuse between the two large conditional branches.

[kamipo]

Using while is a common optimization pattern to avoid each block evaluation.
That is a reason that didn't use each_with_index in this case.

Now, that part is only onece evaluated by resusing the first column hash template, so no longer extremely important the avoiding block evaluation.

[kaspth]

Using while is a common optimization pattern to avoid each block evaluation.

Yeah, totally. It's just that if the block execution isn't slower and doesn't allocate more, then… it's not exactly optimizing that much? In any case I'm fine either way.