rubocop-performance icon indicating copy to clipboard operation
rubocop-performance copied to clipboard

Suggestion from Performance/BlockGivenWithExplicitBlock is often slower

Open jhawthorn opened this issue 1 year ago • 3 comments

Performance/BlockGivenWithExplicitBlock provides bad guidance and should be removed. The benchmark which was given when it was introduced didn't test the case that a block was given, when that happens block is significantly slower than the other option.

For Ruby 3.2+ I made if block faster, but prior to that the advice from this cop was always wrong. Even in 3.2+ I believe it's bad advice as there's only a very narrow usage for which block is (very slightly) faster, and even in those cases it's likely a future refactor the user will accidentally introduce the slow case (ex. moving from if block to if block && something_else)

require 'bundler/inline'

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

def if_block(&block)
  !!block
end

def if_block_given(&block)
  !!block_given?
end

Benchmark.ips do |x|
  x.report('block')                  { if_block }
  x.report('block w/ block')         { if_block {} }
  x.report('block_given?')           { if_block_given }
  x.report('block_given? w/ block')  { if_block_given {} }
  x.compare!
end
Warming up --------------------------------------
            block     1.612M i/100ms
   block w/ block   508.391k i/100ms
     block_given?     1.309M i/100ms
block_given? w/ block
                         1.068M i/100ms
Calculating -------------------------------------
            block     15.804M (± 1.8%) i/s -     79.000M in   5.000346s
   block w/ block      5.017M (± 0.5%) i/s -     25.420M in   5.067106s
     block_given?     13.297M (± 1.7%) i/s -     66.746M in   5.021282s
block_given? w/ block
                         10.745M (± 0.6%) i/s -     54.450M in   5.067787s

Comparison:
            block: 15804072.8 i/s
     block_given?: 13296844.8 i/s - 1.19x  slower
block_given? w/ block: 10744860.7 i/s - 1.47x  slower
   block w/ block:  5016692.5 i/s - 3.15x  slower

jhawthorn avatar Nov 20 '23 18:11 jhawthorn

To be fair, the assumption is that with block_given? you can often avoid declaring &block:

require 'bundler/inline'

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

def if_block(&block)
  !!block
end

def if_block_given
  block_given?
end

Benchmark.ips do |x|
  x.report('block')                  { if_block }
  x.report('block w/ block')         { if_block {} }
  x.report('block_given?')           { if_block_given }
  x.report('block_given? w/ block')  { if_block_given {} }
  x.compare!(order: :baseline)
end
ruby 3.2.2 (2023-03-30 revision e51014f9c0) [arm64-darwin22]
Resolving dependencies...
Warming up --------------------------------------
               block     1.343M i/100ms
      block w/ block   513.029k i/100ms
        block_given?     1.465M i/100ms
block_given? w/ block
                         1.457M i/100ms
Calculating -------------------------------------
               block     13.177M (± 2.0%) i/s -     67.134M in   5.096854s
      block w/ block      5.471M (± 4.3%) i/s -     27.704M in   5.074617s
        block_given?     14.664M (± 0.8%) i/s -     74.736M in   5.096843s
block_given? w/ block
                         14.537M (± 0.8%) i/s -     72.851M in   5.011781s

Comparison:
               block: 13176873.3 i/s
        block_given?: 14664219.5 i/s - 1.11x  faster
block_given? w/ block: 14537095.2 i/s - 1.10x  faster
      block w/ block:  5470680.3 i/s - 2.41x  slower

But even then, the difference is too small to be worth in unless you enter the case where the Proc has to be instantiated.

require 'bundler/inline'

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

def if_block(&block)
  block ? true : false
end

def if_block_given
  block_given?
end

Benchmark.ips do |x|
  x.report('block')                  { if_block }
  x.report('block w/ block')         { if_block {} }
  x.report('block_given?')           { if_block_given }
  x.report('block_given? w/ block')  { if_block_given {} }
  x.compare!(order: :baseline)
end
Warming up --------------------------------------
               block     1.428M i/100ms
      block w/ block     1.321M i/100ms
        block_given?     1.456M i/100ms
block_given? w/ block
                         1.451M i/100ms
Calculating -------------------------------------
               block     14.424M (± 1.6%) i/s -     72.851M in   5.052055s
      block w/ block     13.159M (± 1.4%) i/s -     66.037M in   5.019221s
        block_given?     14.549M (± 1.4%) i/s -     72.786M in   5.003860s
block_given? w/ block
                         14.404M (± 1.3%) i/s -     72.529M in   5.036180s

Comparison:
               block: 14424088.9 i/s
        block_given?: 14548868.5 i/s - same-ish: difference falls within error
block_given? w/ block: 14404275.0 i/s - same-ish: difference falls within error
      block w/ block: 13159479.3 i/s - 1.10x  slower

casperisfine avatar Nov 20 '23 19:11 casperisfine

To be fair, the assumption is that with block_given? you can often avoid declaring &block:

That's another point in favour of removing the linter, which tells you to replace block_given? with block.

jhawthorn avatar Nov 20 '23 19:11 jhawthorn

ROFL, I just assumed it was advocating for block_given?, I couldn't imagine &block would ever be faster. Same perf at best.

So yeah, definite 👍

byroot avatar Nov 20 '23 19:11 byroot