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. It was detected by the following rubocop configuration:

  Performance/StringIdentifierArgument:
    Enabled: true

Apparently, string identifier argument should not be used.

Benchmark result:

  ruby 3.2.8 (2025-03-26 revision 13f495dc2c) +YJIT [x86_64-linux]
  Warming up --------------------------------------
  check method with string notation
                         875.323k i/100ms
  check method with symbol notation
                           1.880M i/100ms
  Calculating -------------------------------------
  check method with string notation
                            8.828M (± 1.4%) i/s  (113.28 ns/i) -     44.641M in   5.058002s
  check method with symbol notation
                           19.766M (± 2.2%) i/s   (50.59 ns/i) -     99.631M in   5.043071s

  Comparison:
  check method with symbol notation: 19765521.3 i/s
  check method with string notation:  8827598.1 i/s - 2.24x  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("check method with string notation") {
    "test".respond_to?("start_with?")
  }
  x.report("check method with symbol notation") {
    "test".respond_to?(:start_with?)
  }
  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/StringIdentifierArgument:
    Enabled: true
```

Apparently, string identifier argument should not be used.

Benchmark result:

```
  ruby 3.2.8 (2025-03-26 revision 13f495dc2c) +YJIT [x86_64-linux]
  Warming up --------------------------------------
  check method with string notation
                         875.323k i/100ms
  check method with symbol notation
                           1.880M i/100ms
  Calculating -------------------------------------
  check method with string notation
                            8.828M (± 1.4%) i/s  (113.28 ns/i) -     44.641M in   5.058002s
  check method with symbol notation
                           19.766M (± 2.2%) i/s   (50.59 ns/i) -     99.631M in   5.043071s

  Comparison:
  check method with symbol notation: 19765521.3 i/s
  check method with string notation:  8827598.1 i/s - 2.24x  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("check method with string notation") {
    "test".respond_to?("start_with?")
  }
  x.report("check method with symbol notation") {
    "test".respond_to?(:start_with?)
  }
  x.compare!
end
```

Signed-off-by: Kentaro Hayashi <[email protected]>
@kenhys kenhys force-pushed the rubocop-respond-to branch from 7842f7a to c0a5274 Compare April 26, 2025 09:02
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 80f3a44 into fluent:master Apr 27, 2025
10 checks passed
@kenhys kenhys deleted the rubocop-respond-to branch April 27, 2025 06:47
@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