Skip to content

Commit 54c9864

Browse files
authored
robocop: reduce string to symbol conversion for performance (#4929)
**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 -------------------------------------- get instance variable with string notation 918.201k i/100ms get instance variable with symbol notation 2.186M i/100ms Calculating ------------------------------------- get instance variable with string notation 10.480M (± 1.5%) i/s (95.42 ns/i) - 53.256M in 5.082903s get instance variable with symbol notation 27.349M (± 1.3%) i/s (36.56 ns/i) - 137.703M in 5.035946s Comparison: get instance variable with symbol notation: 27349072.3 i/s get instance variable with string notation: 10479751.9 i/s - 2.61x slower ruby 3.2.8 (2025-03-26 revision 13f495dc2c) +YJIT [x86_64-linux] Warming up -------------------------------------- defined? instance variable with string notation 919.834k i/100ms defined? instance variable with symbol notation 2.333M i/100ms Calculating ------------------------------------- defined? instance variable with string notation 9.011M (± 3.6%) i/s (110.98 ns/i) - 45.072M in 5.008998s defined? instance variable with symbol notation 27.433M (± 2.2%) i/s (36.45 ns/i) - 137.670M in 5.021011s Comparison: defined? instance variable with symbol notation: 27432875.4 i/s defined? instance variable with string notation: 9010878.0 i/s - 3.04x slower ``` Appendix: benchmark ruby script ```ruby require 'bundler/inline' gemfile do source 'https://rubygems.org' gem 'benchmark-ips' gem 'benchmark-memory' end class Dummy def initialize @running = nil end end Benchmark.ips do |x| @dummy = Dummy.new x.report("get instance variable with string notation") { @dummy.instance_variable_get("@running") } x.report("get instance variable with symbol notation") { @dummy.instance_variable_get(:@running) } x.compare! end Benchmark.ips do |x| @dummy = Dummy.new x.report("defined? instance variable with string notation") { @dummy.instance_variable_defined?("@running") } x.report("defined? instance variable with symbol notation") { @dummy.instance_variable_defined?(:@running) } x.compare! end ``` **Docs Changes**: N/A **Release Note**: N/A Signed-off-by: Kentaro Hayashi <hayashi@clear-code.com>
1 parent 98a4e32 commit 54c9864

File tree

6 files changed

+10
-10
lines changed

6 files changed

+10
-10
lines changed

lib/fluent/compat/socket_util.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ def start
129129

130130
def shutdown
131131
@loop.watchers.each { |w| w.detach }
132-
@loop.stop if @loop.instance_variable_get("@running")
132+
@loop.stop if @loop.instance_variable_get(:@running)
133133
@handler.close
134134
@thread.join
135135

lib/fluent/plugin/owned_by_mixin.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,13 @@ def owner=(plugin)
2626
end
2727

2828
def owner
29-
if instance_variable_defined?("@_owner")
29+
if instance_variable_defined?(:@_owner)
3030
@_owner
3131
end
3232
end
3333

3434
def log
35-
if instance_variable_defined?("@log")
35+
if instance_variable_defined?(:@log)
3636
@log
3737
end
3838
end

lib/fluent/plugin_id.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,13 @@ def plugin_id_for_test?
5757
end
5858

5959
def plugin_id_configured?
60-
if instance_variable_defined?("@_id_configured")
60+
if instance_variable_defined?(:@_id_configured)
6161
@_id_configured
6262
end
6363
end
6464

6565
def plugin_id
66-
if instance_variable_defined?("@id")
66+
if instance_variable_defined?(:@id)
6767
@id || "object:#{object_id.to_s(16)}"
6868
else
6969
"object:#{object_id.to_s(16)}"

lib/fluent/system_config.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,13 +186,13 @@ def system_config
186186
unless defined?($_system_config)
187187
$_system_config = nil
188188
end
189-
(instance_variable_defined?("@_system_config") && @_system_config) ||
189+
(instance_variable_defined?(:@_system_config) && @_system_config) ||
190190
$_system_config || Fluent::Engine.system_config
191191
end
192192

193193
def system_config_override(opts={})
194194
require 'fluent/engine'
195-
if !instance_variable_defined?("@_system_config") || @_system_config.nil?
195+
if !instance_variable_defined?(:@_system_config) || @_system_config.nil?
196196
@_system_config = (defined?($_system_config) && $_system_config ? $_system_config : Fluent::Engine.system_config).dup
197197
end
198198
opts.each_pair do |key, value|

test/compat/test_parser.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ def test_setting_estimate_current_event_value
8686
def test_regexp_parser_config(options)
8787
source = "(?<test>.*)"
8888
parser = Fluent::TextParser::RegexpParser.new(Regexp.new(source, options), { "dummy" => "dummy" })
89-
regexp = parser.instance_variable_get("@regexp")
89+
regexp = parser.instance_variable_get(:@regexp)
9090
assert_equal(options, regexp.options)
9191
end
9292
end

test/test_log.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,7 @@ def test_disable_events
580580
logger = ServerEngine::DaemonLogger.new(logdev, dl_opts)
581581
log = Fluent::Log.new(logger)
582582
log.enable_event(true)
583-
engine = log.instance_variable_get("@engine")
583+
engine = log.instance_variable_get(:@engine)
584584
mock(engine).push_log_event(anything, anything, anything).once
585585
log.trace "trace log"
586586
log.disable_events(Thread.current)
@@ -728,7 +728,7 @@ def teardown
728728

729729
def test_initialize
730730
log = Fluent::PluginLogger.new(@logger)
731-
logger = log.instance_variable_get("@logger")
731+
logger = log.instance_variable_get(:@logger)
732732
assert_equal(logger, @logger)
733733
end
734734

0 commit comments

Comments
 (0)