Performance/StringIdentifierArgument for interpolated string doesn't seem to boost performance.

[mikdiet]

Is your feature request related to a problem? Please describe.

I updated the gem to v1.20.0 and it now suggests me to replace interpolated strings with interpolated symbols.

I did some memory profiling with memory_profiler gem, and it shows me, that interpolated symbols are actually allocate string first:

require "memory_profiler"

MemoryProfiler.report { :'foo_bar' }.pretty_print
# no strings allocated

MemoryProfiler.report { :"foo_#{bar}" }.pretty_print
Allocated String Report
-----------------------------------
         1  "foo_bar"
         1  (irb):23

As you can see, no performance gain, as the string is still in memory, and conversion to symbol will anyway happen, in my code, or in kernel.

Describe the solution you'd like

I'd like interpolated symbols feature to be reverted from Performance/StringIdentifierArgument cop, or be customised with flag.

Describe alternatives you've considered

Disabling a cop is poor option for me, as the rest examples are reasonable.

Additional context

I checked in irb of the latest ruby:

» ruby -v
ruby 3.2.2 (2023-03-30 revision e51014f9c0) [x86_64-darwin21]

[koic]

Change introduced at https://github.com/rubocop/rubocop-performance/issues/386. @Earlopain, do you have any thoughts on this?

[Earlopain]

As per the PR, I did benchmark this change first, slightly modified from the benchmark in the initial introduction of this cop with https://github.com/rubocop/rubocop-performance/pull/276/commits/9a526622cd35edee3d577cb5ba79f570c4eaec36.

The difference I measure for this still isn't much, but repeatable. It makes sense to me that a string will be allocated nontheless since symbols are just string representations and if the string doesn't exist it needs to be created.

Here's the instruction sequence for old/new:

def bar
  "bar"
end

def old
  send("foo_#{bar}")
end

def new
  send(:"foo_#{bar}")
end

puts RubyVM::InstructionSequence.disasm(method(:old))

0000 putself                                                          (  96)[LiCa]
0001 putobject                              "foo_"
0003 putself
0004 opt_send_without_block                 <calldata!mid:bar, argc:0, FCALL|VCALL|ARGS_SIMPLE>
0006 dup
0007 objtostring                            <calldata!mid:to_s, argc:0, FCALL|ARGS_SIMPLE>
0009 anytostring
0010 concatstrings                          2
0012 opt_send_without_block                 <calldata!mid:send, argc:1, FCALL|ARGS_SIMPLE>
0014 leave                                                            (  97)[Re]

----------

puts RubyVM::InstructionSequence.disasm(method(:new))

0000 putself                                                          ( 104)[LiCa]
0001 putobject                              "foo_"
0003 putself
0004 opt_send_without_block                 <calldata!mid:bar, argc:0, FCALL|VCALL|ARGS_SIMPLE>
0006 dup
0007 objtostring                            <calldata!mid:to_s, argc:0, FCALL|ARGS_SIMPLE>
0009 anytostring
0010 concatstrings                          2
0012 intern
0013 opt_send_without_block                 <calldata!mid:send, argc:1, FCALL|ARGS_SIMPLE>
0015 leave                                                            ( 105)[Re]

The difference here is the intern instruction which is only present on the symbol type. I'm not big on what goes on in the RubyVM, all I saw was that this version seems slightly faster. I can guess that there is some internal optimization but I wouldn't be able to point my at something specific

I did tweak the benchmark script somewhat in case I did something wrong there but I still get similar results:

require 'benchmark/ips'

puts `ruby -v`

def foo_bar
end

def bar
  "bar"
end

def bar_dup
  "bar".dup
end

def old
  send("foo_#{bar}")
end

def old_dup
  send("foo_#{bar_dup}")
end

def new
  send(:"foo_#{bar}")
end

def new_dup
  send(:"foo_#{bar_dup}")
end

Benchmark.ips do |x|
  x.report('string arg') { old }
  x.report('symbol arg') { new }

  x.compare!
end

Warming up --------------------------------------
          string arg   234.366k i/100ms
          symbol arg   202.525k i/100ms
Calculating -------------------------------------
          string arg      2.319M (± 9.4%) i/s -     11.718M in   5.105573s
          symbol arg      2.559M (± 8.0%) i/s -     12.759M in   5.020355s

Comparison:
          symbol arg:  2559051.2 i/s
          string arg:  2319284.6 i/s - same-ish: difference falls within error

Benchmark.ips do |x|
  x.report('string arg dup') { old_dup }
  x.report('symbol arg dup') { new_dup }

  x.compare!
end

Warming up --------------------------------------
      string arg dup   136.329k i/100ms
      symbol arg dup   147.234k i/100ms
Calculating -------------------------------------
      string arg dup      1.236M (±16.8%) i/s -      5.998M in   5.030565s
      symbol arg dup      1.418M (± 7.6%) i/s -      7.067M in   5.015928s

Comparison:
      symbol arg dup:  1417551.6 i/s
      string arg dup:  1235997.1 i/s - same-ish: difference falls within error

Benchmark says it's same-ish, but consistently so in favor of symbol.