Handle map { ... }.sum in Performance/Sum

[eugeneius]

Followup to #137.

Passing the block directly to sum is faster, and avoids allocating an intermediate array.

Benchmark

# frozen_string_literal: true

require 'bundler/inline'

gemfile(true) do
  gem 'benchmark-ips'
  gem 'benchmark-memory'
end

SCENARIOS = [
  1..10,
  1..1_000,
  1..100_000
]

SCENARIOS.each do |range|
  puts
  puts " #{range.inspect} ".center(80, "=")
  puts

  Benchmark.ips do |x|
    x.report("map { |n| n ** 2 }.sum") { range.map { |n| n ** 2 }.sum }
    x.report("sum { |n| n ** 2 }")     { range.sum { |n| n ** 2 } }

    x.compare!
  end

  Benchmark.memory do |x|
    x.report("map { |n| n ** 2 }.sum") { range.map { |n| n ** 2 }.sum }
    x.report("sum { |n| n ** 2 }")     { range.sum { |n| n ** 2 } }

    x.compare!
  end
end

Results

==================================== 1..10 =====================================

Warming up --------------------------------------
map { |n| n ** 2 }.sum
                        84.528k i/100ms
  sum { |n| n ** 2 }    97.509k i/100ms
Calculating -------------------------------------
map { |n| n ** 2 }.sum
                          1.015M (± 7.9%) i/s -      5.072M in   5.035982s
  sum { |n| n ** 2 }      1.225M (± 5.9%) i/s -      6.143M in   5.042741s

Comparison:
  sum { |n| n ** 2 }:  1224975.6 i/s
map { |n| n ** 2 }.sum:  1014535.8 i/s - 1.21x  slower

Calculating -------------------------------------
map { |n| n ** 2 }.sum
                       200.000  memsize (     0.000  retained)
                         1.000  objects (     0.000  retained)
                         0.000  strings (     0.000  retained)
  sum { |n| n ** 2 }     0.000  memsize (     0.000  retained)
                         0.000  objects (     0.000  retained)
                         0.000  strings (     0.000  retained)

Comparison:
  sum { |n| n ** 2 }:          0 allocated
map { |n| n ** 2 }.sum:        200 allocated - Infx more

=================================== 1..1000 ====================================

Warming up --------------------------------------
map { |n| n ** 2 }.sum
                         1.233k i/100ms
  sum { |n| n ** 2 }     1.446k i/100ms
Calculating -------------------------------------
map { |n| n ** 2 }.sum
                         13.256k (± 3.5%) i/s -     66.582k in   5.030415s
  sum { |n| n ** 2 }     14.734k (± 1.3%) i/s -     73.746k in   5.005859s

Comparison:
  sum { |n| n ** 2 }:    14734.4 i/s
map { |n| n ** 2 }.sum:    13256.1 i/s - 1.11x  slower

Calculating -------------------------------------
map { |n| n ** 2 }.sum
                        11.840k memsize (     0.000  retained)
                         1.000  objects (     0.000  retained)
                         0.000  strings (     0.000  retained)
  sum { |n| n ** 2 }     0.000  memsize (     0.000  retained)
                         0.000  objects (     0.000  retained)
                         0.000  strings (     0.000  retained)

Comparison:
  sum { |n| n ** 2 }:          0 allocated
map { |n| n ** 2 }.sum:      11840 allocated - Infx more

================================== 1..100000 ===================================

Warming up --------------------------------------
map { |n| n ** 2 }.sum
                        12.000  i/100ms
  sum { |n| n ** 2 }    14.000  i/100ms
Calculating -------------------------------------
map { |n| n ** 2 }.sum
                        132.430  (± 2.3%) i/s -    672.000  in   5.076442s
  sum { |n| n ** 2 }    147.826  (± 2.0%) i/s -    742.000  in   5.021156s

Comparison:
  sum { |n| n ** 2 }:      147.8 i/s
map { |n| n ** 2 }.sum:      132.4 i/s - 1.12x  slower

Calculating -------------------------------------
map { |n| n ** 2 }.sum
                         1.022M memsize (     0.000  retained)
                         1.000  objects (     0.000  retained)
                         0.000  strings (     0.000  retained)
  sum { |n| n ** 2 }     0.000  memsize (     0.000  retained)
                         0.000  objects (     0.000  retained)
                         0.000  strings (     0.000  retained)

Comparison:
  sum { |n| n ** 2 }:          0 allocated
map { |n| n ** 2 }.sum:    1021592 allocated - Infx more

---

Before submitting the PR make sure the following are checked:

• Wrote good commit messages.
• Commit message starts with [Fix #issue-number] (if the related issue exists).
• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
• The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
• Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.

[koic]

Thank you!

[shepmaster]

This looks to have issues with foo.map(&:bar).sum(initial_value), which is corrected to foo.sum(initial_value)(&:bar)

[eugeneius]

Thanks for reporting that @shepmaster, I've opened #172 to fix it.

[shepmaster]

This looks to have a false positive when used with Active Record:

User.limit(10).map(&:id).sum(10)
# => 141467

User.limit(10).sum(10, &:id)
# ArgumentError: Column name argument is not supported when a block is passed.
# from .../activerecord-6.0.3.3/lib/active_record/relation/calculations.rb:87:in `sum'

[marcandre]

This looks to have a false positive when used with Active Record:

User.limit(10).map(&:id).sum(10)
# => 141467

User.limit(10).sum(10, &:id)
# ArgumentError: Column name argument is not supported when a block is passed.
# from .../activerecord-6.0.3.3/lib/active_record/relation/calculations.rb:87:in `sum'

More like an unsafe correction. The code should still be modified, e.g. User.limit(10).sum(:id) + 10