Skip to content

Conversation

@kenhys
Copy link
Contributor

@kenhys kenhys commented Apr 27, 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

string identifier argument should not be used.

Benchmark result:

  ruby 3.2.8 (2025-03-26 revision 13f495dc2c) +YJIT [x86_64-linux]
  Warming up --------------------------------------
  define method with string notation
                         225.825k i/100ms
  define method with symbol notation
                         278.180k i/100ms
  Calculating -------------------------------------
  define method with string notation
                            2.337M (± 2.0%) i/s  (427.83 ns/i) -     11.743M in   5.025989s
  define method with symbol notation
                            2.765M (± 3.8%) i/s  (361.70 ns/i) -     13.909M in   5.038885s

  Comparison:
  define method with symbol notation:  2764687.4 i/s
  define method with string notation:  2337378.7 i/s - 1.18x  slower

Appendix: benchmark script

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

class Dummy
end

Benchmark.ips do |x|
  @dummy = Dummy.new
  x.report("define method with string notation") {
    @dummy.define_singleton_method("name") { "class name" }
  }
  x.report("define method with symbol notation") {
    @dummy.define_singleton_method(:name) { "class name" }
  }
  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
```

string identifier argument should not be used.

Benchmark result:

```
  ruby 3.2.8 (2025-03-26 revision 13f495dc2c) +YJIT [x86_64-linux]
  Warming up --------------------------------------
  define method with string notation
                         225.825k i/100ms
  define method with symbol notation
                         278.180k i/100ms
  Calculating -------------------------------------
  define method with string notation
                            2.337M (± 2.0%) i/s  (427.83 ns/i) -     11.743M in   5.025989s
  define method with symbol notation
                            2.765M (± 3.8%) i/s  (361.70 ns/i) -     13.909M in   5.038885s

  Comparison:
  define method with symbol notation:  2764687.4 i/s
  define method with string notation:  2337378.7 i/s - 1.18x  slower
```

Appendix: benchmark script

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

class Dummy
end

Benchmark.ips do |x|
  @dummy = Dummy.new
  x.report("define method with string notation") {
    @dummy.define_singleton_method("name") { "class name" }
  }
  x.report("define method with symbol notation") {
    @dummy.define_singleton_method(:name) { "class name" }
  }
  x.compare!
end
```

Signed-off-by: Kentaro Hayashi <[email protected]>
@kenhys kenhys force-pushed the rubocop-define-singleton-method branch from 30efdd5 to ee65d44 Compare April 27, 2025 07:12
@kenhys kenhys merged commit 1f39f41 into fluent:master Apr 27, 2025
10 checks passed
@kenhys kenhys deleted the rubocop-define-singleton-method branch April 27, 2025 07:54
@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