From 8907a128a7ed39589c539e39997ac270f322c5bc Mon Sep 17 00:00:00 2001 From: Earlopain <14981592+Earlopain@users.noreply.github.com> Date: Fri, 13 Sep 2024 10:54:22 +0200 Subject: [PATCH] [Fix #385] Disable `Performance/BlockGivenWithExplicitBlock` by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See linked issue for benchmark details. I tested with YJIT and its even worse: ``` ruby 3.4.0dev (2024-09-05T09:43:46Z master 63cbe3f6ac) +YJIT [x86_64-linux] Warming up -------------------------------------- block 1.635M i/100ms block w/ block 285.930k i/100ms block_given? 1.867M i/100ms block_given? w/ block 1.497M i/100ms Calculating ------------------------------------- block 22.611M (±12.7%) i/s (44.23 ns/i) - 111.180M in 5.033721s block w/ block 3.034M (± 8.5%) i/s (329.59 ns/i) - 15.154M in 5.032609s block_given? 24.935M (±10.7%) i/s (40.10 ns/i) - 123.254M in 5.002983s block_given? w/ block 23.007M (±14.7%) i/s (43.47 ns/i) - 113.782M in 5.058213s Comparison: block_given?: 24935388.3 i/s block_given? w/ block: 23006920.5 i/s - same-ish: difference falls within error block: 22610939.8 i/s - same-ish: difference falls within error block w/ block: 3034047.2 i/s - 8.22x slower ``` --- changelog/change_disable_block_given.md | 1 + config/default.yml | 5 ++++- .../cop/performance/block_given_with_explicit_block.rb | 3 +++ 3 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 changelog/change_disable_block_given.md diff --git a/changelog/change_disable_block_given.md b/changelog/change_disable_block_given.md new file mode 100644 index 0000000000..535858d8cb --- /dev/null +++ b/changelog/change_disable_block_given.md @@ -0,0 +1 @@ +* [#385](https://github.com/rubocop/rubocop-performance/issues/385): Disable `Performance/BlockGivenWithExplicitBlock` by default. ([@earlopain][]) diff --git a/config/default.yml b/config/default.yml index 95dc830898..cf44ddf5ed 100644 --- a/config/default.yml +++ b/config/default.yml @@ -32,8 +32,11 @@ Performance/BindCall: Performance/BlockGivenWithExplicitBlock: Description: 'Check block argument explicitly instead of using `block_given?`.' - Enabled: pending + # This cop was created due to a mistake in microbenchmark. + # https://github.com/rubocop/rubocop-performance/issues/385 + Enabled: false VersionAdded: '1.9' + VersionChanged: <> Performance/Caller: Description: >- diff --git a/lib/rubocop/cop/performance/block_given_with_explicit_block.rb b/lib/rubocop/cop/performance/block_given_with_explicit_block.rb index a3d804694a..4fc44169ef 100644 --- a/lib/rubocop/cop/performance/block_given_with_explicit_block.rb +++ b/lib/rubocop/cop/performance/block_given_with_explicit_block.rb @@ -6,6 +6,9 @@ module Performance # Identifies unnecessary use of a `block_given?` where explicit check # of block argument would suffice. # + # NOTE: This cop produces code with significantly worse performance when a + # block is being passed to the method and as such should not be enabled. + # # @example # # bad # def method(&block)