Performance/UnfreezeString should be disabled or heavilly changed in Ruby 3.3+

[byroot]

As of ruby/ruby#8952, +"foo" and "foo".dup have similar performance, but the later doesn't require parentheses for chaining, and is much more obvious.

So IMHO rubocop-performance should advocate for .dup over +@ when applicable.

[apoorv1316]

Hi @koic, Can I work on this when we have a stable release for 3.3?

[koic]

RuboCop Performance prioritizes speed over readability. This means that in cases of trade-off, it opts for speed. While it's confirmed that String#dup has been optimized in Ruby 3.3, it seems String#+@ still tends to be faster:

# unfreeze_string.rb
require 'bundler/inline'

gemfile(true) do
  gem 'benchmark-ips'
end

Benchmark.ips do |x|
  x.report("''.dup") { ''.dup }
  x.report('"something".dup') { 'something'.dup }
  x.report('String.new') { String.new }
  x.report("String.new('')") { String.new('') }
  x.report("String.new('something')") { String.new('something') }
  x.report("+'something'") { +'something' }
  x.report("+''") { +'' }

  x.compare!
end

Ruby 3.2.2

$ ruby -v
ruby 3.2.2 (2023-03-30 revision e51014f9c0) [x86_64-darwin19]

$ ruby unfreeze_string.rb
(snip)

Warming up --------------------------------------
              ''.dup   391.873k i/100ms
     "something".dup   380.524k i/100ms
          String.new   642.631k i/100ms
      String.new('')   433.390k i/100ms
String.new('something')
                       406.262k i/100ms
        +'something'   969.512k i/100ms
                 +''   796.133k i/100ms
Calculating -------------------------------------
              ''.dup      3.423M (± 6.4%) i/s -     17.242M in   5.060122s
     "something".dup      3.671M (± 3.2%) i/s -     18.646M in   5.084613s
          String.new      6.303M (± 3.3%) i/s -     32.132M in   5.103560s
      String.new('')      4.343M (± 4.5%) i/s -     21.670M in   5.000633s
String.new('something')
                          4.315M (± 1.9%) i/s -     21.938M in   5.086583s
        +'something'     10.068M (± 2.1%) i/s -     50.415M in   5.009757s
                 +''     10.320M (± 3.0%) i/s -     51.749M in   5.018752s

Comparison:
                 +'': 10320487.3 i/s
        +'something': 10067912.9 i/s - same-ish: difference falls within error
          String.new:  6302816.7 i/s - 1.64x  slower
      String.new(''):  4343128.0 i/s - 2.38x  slower
String.new('something'):  4314556.0 i/s - 2.39x  slower
     "something".dup:  3670993.7 i/s - 2.81x  slower
              ''.dup:  3422727.6 i/s - 3.02x  slower

Ruby 3.3.0dev

$ ruby -v
ruby 3.3.0dev (2023-11-27T06:04:40Z master 71a8daecd9) [x86_64-darwin22]

$ ruby unfreeze_string.rb
(snip)

Warming up --------------------------------------
              ''.dup   762.805k i/100ms
     "something".dup   765.943k i/100ms
          String.new   634.272k i/100ms
      String.new('')   438.438k i/100ms
String.new('something')
                       412.263k i/100ms
        +'something'     1.027M i/100ms
                 +''     1.041M i/100ms
Calculating -------------------------------------
              ''.dup      7.103M (± 6.0%) i/s -     35.852M in   5.068217s
     "something".dup      7.306M (± 5.6%) i/s -     36.765M in   5.050130s
          String.new      5.965M (± 5.1%) i/s -     29.811M in   5.010758s
      String.new('')      3.958M (± 4.5%) i/s -     20.168M in   5.105479s
String.new('something')
                          3.935M (± 4.2%) i/s -     19.789M in   5.038066s
        +'something'      9.889M (± 7.7%) i/s -     49.306M in   5.019341s
                 +''      9.561M (± 6.2%) i/s -     47.907M in   5.030674s

Comparison:
        +'something':  9889374.1 i/s
                 +'':  9560610.7 i/s - same-ish: difference falls within error
     "something".dup:  7305892.1 i/s - 1.35x  slower
              ''.dup:  7102525.5 i/s - 1.39x  slower
          String.new:  5965012.8 i/s - 1.66x  slower
      String.new(''):  3957910.8 i/s - 2.50x  slower
String.new('something'):  3934515.1 i/s - 2.51x  slower

It's possible to mention the optimization in Ruby 3.3 in the documentation, but it likely wouldn't be sufficient reason to disabled by default or retire the cop.

[byroot]

@koic the reason it's faster in your benchmark is that +@ is essentially str.frozen? ? str.dup : str.

And the benchmark you posted doesn't have # frozen_string_literal: true. If you set it, the perf will be the same (very sligthly faster because it doesn't have to check for frozen state).

[koic]

Ah, I see! I've added fstring magic comment and redone the benchmarks:

$ cat unfreeze_string.rb
# frozen_string_literal: true

require 'bundler/inline'

gemfile(true) do
  gem 'benchmark-ips'
end

Benchmark.ips do |x|
  x.report("''.dup") { ''.dup }
  x.report('"something".dup') { 'something'.dup }
  x.report('String.new') { String.new }
  x.report("String.new('')") { String.new('') }
  x.report("String.new('something')") { String.new('something') }
  x.report("+'something'") { +'something' }
  x.report("+''") { +'' }

  x.compare!
end

Ruby 3.2.2

$ ruby unfreeze_string.rb
(snip)

Warming up --------------------------------------
              ''.dup   513.181k i/100ms
     "something".dup   480.564k i/100ms
          String.new   686.240k i/100ms
      String.new('')   609.997k i/100ms
String.new('something')
                       596.819k i/100ms
        +'something'     1.114M i/100ms
                 +''     1.115M i/100ms
Calculating -------------------------------------
              ''.dup      5.141M (± 2.8%) i/s -     26.172M in   5.095203s
     "something".dup      5.109M (± 2.3%) i/s -     25.950M in   5.082170s
          String.new      7.108M (± 2.2%) i/s -     35.684M in   5.022839s
      String.new('')      5.996M (± 2.0%) i/s -     30.500M in   5.088796s
String.new('something')
                          5.921M (± 1.6%) i/s -     29.841M in   5.040809s
        +'something'     10.889M (± 4.9%) i/s -     54.590M in   5.028321s
                 +''     10.975M (± 2.4%) i/s -     55.762M in   5.084026s

Comparison:
                 +'': 10974731.2 i/s
        +'something': 10888592.5 i/s - same-ish: difference falls within error
          String.new:  7108016.9 i/s - 1.54x  slower
      String.new(''):  5996034.2 i/s - 1.83x  slower
String.new('something'):  5921472.1 i/s - 1.85x  slower
              ''.dup:  5140885.9 i/s - 2.13x  slower
     "something".dup:  5108906.1 i/s - 2.15x  slower

Ruby 3.3.0dev

% ruby unfreeze_string.rb
(snip)

Warming up --------------------------------------
              ''.dup     1.078M i/100ms
     "something".dup     1.065M i/100ms
          String.new   596.049k i/100ms
      String.new('')   530.120k i/100ms
String.new('something')
                       527.086k i/100ms
        +'something'     1.018M i/100ms
                 +''   938.782k i/100ms
Calculating -------------------------------------
              ''.dup      9.326M (±12.7%) i/s -     46.334M in   5.098238s
      "something".dup      9.423M (± 7.3%) i/s -     47.910M in   5.111233s
            String.new      5.732M (± 9.5%) i/s -     28.610M in   5.041625s
         String.new('')      4.976M (± 5.7%) i/s -     24.916M in   5.023497s
String.new('something')
                          4.942M (± 6.7%) i/s -     24.773M in   5.039283s
         +'something'     10.245M (± 4.7%) i/s -     51.902M in   5.079663s
                   +''     10.170M (± 3.3%) i/s -     51.633M in   5.083187s

Comparison:
        +'something': 10245374.7 i/s
                 +'': 10169970.6 i/s - same-ish: difference falls within error
     "something".dup:  9422907.1 i/s - same-ish: difference falls within error
              ''.dup:  9326320.2 i/s - same-ish: difference falls within error
          String.new:  5731872.5 i/s - 1.79x  slower
      String.new(''):  4976318.6 i/s - 2.06x  slower
String.new('something'):  4941573.7 i/s - 2.07x  slower

Probably for Ruby 3.3 and later, it would be better to extend the cop to allow String#dup, taking into account the presence of fstring magic comment.

[byroot]

Thank you @koic !