New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change Enumerable#sum to use inject(:sym) specification #22336

Merged
merged 1 commit into from Nov 19, 2015

Conversation

Projects
None yet
2 participants
@tjschuck
Contributor

tjschuck commented Nov 18, 2015

Not only does this make for simpler, more obvious code, it's also more performant:

require 'benchmark/ips'

module Enumerable
  def old_sum(identity = 0, &block)
    if block_given?
      map(&block).old_sum(identity)
    else
      inject { |sum, element| sum + element } || identity
    end
  end

  def new_sum(identity = 0, &block)
    if block_given?
      map(&block).new_sum(identity)
    else
      inject(:+) || identity
    end
  end
end

summable = (1..100).to_a # sum is 5050

Benchmark.ips do |x|
  x.report("old_sum") { summable.old_sum }
  x.report("new_sum") { summable.new_sum }
  x.compare!
end

# Calculating -------------------------------------
#              old_sum    10.674k i/100ms
#              new_sum    14.542k i/100ms
# -------------------------------------------------
#              old_sum    117.350k (± 7.1%) i/s -    587.070k
#              new_sum    154.712k (± 3.8%) i/s -    785.268k
#
# Comparison:
#              new_sum:   154712.1 i/s
#              old_sum:   117350.0 i/s - 1.32x slower

More benchmarks here, including summing strings and passing blocks. The performance gains are less for those, but this version still always wins.

@kaspth

This comment has been minimized.

Member

kaspth commented Nov 19, 2015

Nice! Can you amend your commit to add the benchmark to the commit message? That way we don't lose that context later, or have to go to GitHub for it 😄

@kaspth kaspth self-assigned this Nov 19, 2015

Change Enumerable#sum to use inject(:sym) specification
Not only does this make for simpler, more obvious code, it's also more performant:

    require 'benchmark/ips'

    module Enumerable
      def old_sum(identity = 0, &block)
        if block_given?
          map(&block).old_sum(identity)
        else
          inject { |sum, element| sum + element } || identity
        end
      end

      def new_sum(identity = 0, &block)
        if block_given?
          map(&block).new_sum(identity)
        else
          inject(:+) || identity
        end
      end
    end

    summable = (1..100).to_a # sum is 5050

    Benchmark.ips do |x|
      x.report("old_sum") { summable.old_sum }
      x.report("new_sum") { summable.new_sum }
      x.compare!
    end

    # Calculating -------------------------------------
    #              old_sum    10.674k i/100ms
    #              new_sum    14.542k i/100ms
    # -------------------------------------------------
    #              old_sum    117.350k (± 7.1%) i/s -    587.070k
    #              new_sum    154.712k (± 3.8%) i/s -    785.268k
    #
    # Comparison:
    #              new_sum:   154712.1 i/s
    #              old_sum:   117350.0 i/s - 1.32x slower

More benchmarks [here](https://gist.github.com/tjschuck/b3fe4e8c812712376648), including summing strings and passing blocks.  The performance gains are less for those, but this version still always wins.

@tjschuck tjschuck force-pushed the tjschuck:enumerable_sum_perf branch to c703c39 Nov 19, 2015

@tjschuck

This comment has been minimized.

Contributor

tjschuck commented Nov 19, 2015

@kaspth Done!

kaspth added a commit that referenced this pull request Nov 19, 2015

Merge pull request #22336 from tjschuck/enumerable_sum_perf
Change Enumerable#sum to use inject(:sym) specification

@kaspth kaspth merged commit 7e200f2 into rails:master Nov 19, 2015

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
@kaspth

This comment has been minimized.

Member

kaspth commented Nov 19, 2015

Unrelated failure. Thanks! 👍

@tjschuck tjschuck deleted the tjschuck:enumerable_sum_perf branch Nov 19, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment