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

Only create an array with default options if we have default options #32326

Merged
merged 1 commit into from Apr 18, 2018

Conversation

Projects
None yet
2 participants
@oniofchaos
Contributor

oniofchaos commented Mar 23, 2018

Summary

If the options passed in don't have a default key, there's no point in
creating an array from those empty results when we can just go straight
to creating an empty array.

Other Information

Benchmarks:

master_version with false
{:FREE=>-2497, :T_STRING=>52, :T_ARRAY=>2000, :T_HASH=>1000, :T_IMEMO=>1}
master_version with true
{:FREE=>-3001, :T_ARRAY=>2000, :T_HASH=>1000}
fast_version with false
{:FREE=>-1001, :T_ARRAY=>1000}
fast_version with true
{:FREE=>-3001, :T_ARRAY=>2000, :T_HASH=>1000}
Warming up --------------------------------------
master_version with false
                       104.985k i/100ms
master_version with true
                       118.737k i/100ms
fast_version with false
                       206.013k i/100ms
fast_version with true
                       107.005k i/100ms
Calculating -------------------------------------
master_version with false
                          1.970M (±24.6%) i/s -      8.924M in   5.010302s
master_version with true
                          2.152M (±12.4%) i/s -     10.686M in   5.051588s
fast_version with false
                          5.613M (±19.6%) i/s -     26.782M in   5.003740s
fast_version with true
                          2.027M (±15.8%) i/s -      9.951M in   5.065670s

Comparison:
fast_version with false:  5613159.2 i/s
master_version with true:  2152354.4 i/s - 2.61x  slower
fast_version with true:  2027296.0 i/s - 2.77x  slower
master_version with false:  1969824.9 i/s - 2.85x  slower

Benchmark code:

begin
  require "bundler/inline"
rescue LoadError => e
  $stderr.puts "Bundler version 1.10 or later is required. Please update
                your Bundler"
  raise e
end

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

  gem "benchmark-ips"
  gem "rails"
end

def allocate_count
  GC.disable
  before = ObjectSpace.count_objects
  yield
  after = ObjectSpace.count_objects
  after.each { |k,v| after[k] = v - before[k] }
  after[:T_HASH] -= 1 # probe effect - we created the before hash.
  GC.enable
  result = after.reject { |k,v| v == 0 }
  GC.start
  result
end

def master_version(key)
  Array({}.delete(:default)).compact
end

def fast_version(key)
  if key
    Array({}.delete(:default)).compact
  else
    []
  end
end

def test
  puts "master_version with false"
  puts allocate_count { 1000.times { master_version(false) } }
  puts "master_version with true"
  puts allocate_count { 1000.times { master_version(true) } }
  puts "fast_version with false"
  puts allocate_count { 1000.times { fast_version(false) } }
  puts "fast_version with true"
  puts allocate_count { 1000.times { fast_version(true) } }

  Benchmark.ips do |x|
    x.report("master_version with false")  { master_version(false) }
    x.report("master_version with true") { master_version(true) }
    x.report("fast_version with false")    { fast_version(false) }
    x.report("fast_version with true")   { fast_version(true) }
    x.compare!
  end
end

test
Only create an array with default options if we have default options
If the options passed in don't have a default key, there's no point in
creating an array from those empty results when we can just go straight
to creating an empty array.

Benchmarks:
```ruby
master_version with false
{:FREE=>-2497, :T_STRING=>52, :T_ARRAY=>2000, :T_HASH=>1000, :T_IMEMO=>1}
master_version with true
{:FREE=>-3001, :T_ARRAY=>2000, :T_HASH=>1000}
fast_version with false
{:FREE=>-1001, :T_ARRAY=>1000}
fast_version with true
{:FREE=>-3001, :T_ARRAY=>2000, :T_HASH=>1000}
Warming up --------------------------------------
master_version with false
                       104.985k i/100ms
master_version with true
                       118.737k i/100ms
fast_version with false
                       206.013k i/100ms
fast_version with true
                       107.005k i/100ms
Calculating -------------------------------------
master_version with false
                          1.970M (±24.6%) i/s -      8.924M in   5.010302s
master_version with true
                          2.152M (±12.4%) i/s -     10.686M in   5.051588s
fast_version with false
                          5.613M (±19.6%) i/s -     26.782M in   5.003740s
fast_version with true
                          2.027M (±15.8%) i/s -      9.951M in   5.065670s

Comparison:
fast_version with false:  5613159.2 i/s
master_version with true:  2152354.4 i/s - 2.61x  slower
fast_version with true:  2027296.0 i/s - 2.77x  slower
master_version with false:  1969824.9 i/s - 2.85x  slower
```

Benchmark code:
```ruby
begin
  require "bundler/inline"
rescue LoadError => e
  $stderr.puts "Bundler version 1.10 or later is required. Please update
                your Bundler"
  raise e
end

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

  gem "benchmark-ips"
  gem "rails"
end

def allocate_count
  GC.disable
  before = ObjectSpace.count_objects
  yield
  after = ObjectSpace.count_objects
  after.each { |k,v| after[k] = v - before[k] }
  after[:T_HASH] -= 1 # probe effect - we created the before hash.
  GC.enable
  result = after.reject { |k,v| v == 0 }
  GC.start
  result
end

def master_version(key)
  Array({}.delete(:default)).compact
end

def fast_version(key)
  if key
    Array({}.delete(:default)).compact
  else
    []
  end
end

def test
  puts "master_version with false"
  puts allocate_count { 1000.times { master_version(false) } }
  puts "master_version with true"
  puts allocate_count { 1000.times { master_version(true) } }
  puts "fast_version with false"
  puts allocate_count { 1000.times { fast_version(false) } }
  puts "fast_version with true"
  puts allocate_count { 1000.times { fast_version(true) } }

  Benchmark.ips do |x|
    x.report("master_version with false")  { master_version(false) }
    x.report("master_version with true") { master_version(true) }
    x.report("fast_version with false")    { fast_version(false) }
    x.report("fast_version with true")   { fast_version(true) }
    x.compare!
  end
end

test
```
@oniofchaos

This comment has been minimized.

Contributor

oniofchaos commented Apr 15, 2018

r?

@oniofchaos

This comment has been minimized.

Contributor

oniofchaos commented Apr 15, 2018

@guilleiguaran guilleiguaran merged commit 7bcb04c into rails:master Apr 18, 2018

2 checks passed

codeclimate All good!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@oniofchaos oniofchaos deleted the q-centrix:perf-improvement-translation-helper-default-array branch Apr 18, 2018

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