Skip to content

Conversation

kenhys
Copy link
Contributor

@kenhys kenhys commented Apr 26, 2025

Which issue(s) this PR fixes:
Fixes #

What this PR does / why we need it:

This is cosmetic change, it does not change behavior at all.
The following rubocop configuration detected it:

  Performance/CollectionLiteralInLoop:
    Enabled: true

hash literals in loops should not be used.

NOTE: hash literals in loops exists in test/*, but it is overkill to
fix them.

Benchmark result:

ruby 3.2.8 (2025-03-26 revision 13f495dc2c) +YJIT [x86_64-linux]
Warming up --------------------------------------
            in loops   550.000 i/100ms
        out of loops     1.464k i/100ms
Calculating -------------------------------------
            in loops      5.452k (± 1.5%) i/s  (183.43 μs/i) -     27.500k in   5.045506s
        out of loops     14.592k (± 1.0%) i/s   (68.53 μs/i) -     73.200k in   5.016839s

Comparison:
        out of loops:    14592.4 i/s
            in loops:     5451.7 i/s - 2.68x  slower

Appendix: benchmark script

require 'bundler/inline'
gemfile do
  source 'https://rubygems.org'
  gem 'benchmark-ips'
  gem 'benchmark-memory'
end

Benchmark.ips do |x|
  x.report("in loops") {
    1000.times do |n|
      ['source', 'match', 'filter', 'label'].include?("dummy")
    end
  }
  x.report("out of loops") {
    supported_directives = ['source', 'match', 'filter', 'label']
    1000.times do |n|
      supported_directives.include?("dummy")
    end
  }
  x.compare!
end

Docs Changes:

N/A

Release Note:

N/A

This is cosmetic change, it does not change behavior at all.
It was detected by the following rubocop configuration:

```
  Performance/CollectionLiteralInLoop:
    Enabled: true
```

hash literals in loops should not be used.

NOTE: hash literals in loops exists in test/*, but it is overkill to
fix them.

Benchmark result:

```
ruby 3.2.8 (2025-03-26 revision 13f495dc2c) +YJIT [x86_64-linux]
Warming up --------------------------------------
            in loops     1.474k i/100ms
        out of loops   543.000 i/100ms
Calculating -------------------------------------
            in loops     14.481k (± 1.5%) i/s   (69.06 μs/i) -     73.700k in   5.090628s
        out of loops      5.446k (± 1.4%) i/s  (183.63 μs/i) -     27.693k in   5.086275s

Comparison:
            in loops:    14480.8 i/s
        out of loops:     5445.7 i/s - 2.66x  slower
```

Appendix: benchmark script

```ruby
require 'bundler/inline'
gemfile do
  source 'https://rubygems.org'
  gem 'benchmark-ips'
  gem 'benchmark-memory'
end

Benchmark.ips do |x|
  x.report("in loops") {
    supported_directives = ['source', 'match', 'filter', 'label']
    1000.times do |n|
      supported_directives.include?("dummy")
    end
  }
  x.report("out of loops") {
    1000.times do |n|
      ['source', 'match', 'filter', 'label'].include?("dummy")
    end
  }
  x.compare!
end
```

Signed-off-by: Kentaro Hayashi <hayashi@clear-code.com>
Copy link
Contributor

@Watson1978 Watson1978 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻

@kenhys kenhys merged commit 30ae991 into fluent:master Apr 27, 2025
18 of 19 checks passed
@kenhys kenhys deleted the rubocop-collection-literal-in-loop branch April 27, 2025 07:12
@daipom daipom added this to the v1.19.0 milestone Apr 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants