rubocop-performance icon indicating copy to clipboard operation
rubocop-performance copied to clipboard

Request: Object Shape (ivar initialization order) check

Open schneems opened this issue 4 months ago • 2 comments

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

A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]

In Ruby 3.2+ there's a concept of "object shapes" where the order of initialization of instance variables affects performance of Ruby code https://www.youtube.com/watch?v=0lg9Y8gj3FI. The short version is that if you have code that looks like this:

class MyClass
  ...

  def foo
    @foo ||= get_foo
  end
  
  def bar
    @bar ||= get_bar
  end
end

It will generate two different "object shapes" based on which method is called first. The solution is to initialize those values consistently up front

class MyClass
  def initialize(...)
     @foo = nil
     @bar = nil
  end
  # ...
end

With this updated code, all instances of this class will have the same object shape and it's better for performance.

Describe the solution you'd like

Scan code for ivar creation that's not in an initialize method, verify that somewhere in the code, that same named ivar is also set to nil in an initialize method.

I know this gets hairy fast due to the flexability of Ruby, perhaps we make two rules "obviously bad usage" and "very agressive warnings that you might have to disable a lot" and only enable the "obviously bad usage" by default.

Describe alternatives you've considered

Manually scanning. But a lot of people don't know about it. See https://github.com/puma/puma/pull/3714 for an IRL fix.

Additional context

Robocop meme reference!

Image

schneems avatar Aug 29 '25 14:08 schneems

Having some shapes it not such a big deal. It only gets somewhat troublesome if you have too many (currently 8). Consider this benchmark:

require 'bundler/inline'

gemfile do
  gem 'benchmark-ips'
end

require 'benchmark/ips'

Warning[:performance] = true

class SingleShape
  def initialize(ivar = nil)
    return unless ivar

    create_random_ivars
    instance_variable_set(ivar, 123)
  end

  def create_random_ivars
    (1..10).to_a.shuffle.each { instance_variable_set("@ivar#{it}", 123) }
  end

  def access
    @foo
    @foo
    @foo
    @foo
    @foo
    @foo
    @foo
    @foo
    @foo
    @foo
  end
end

class FiveShapes < SingleShape
end

class TooComplex < SingleShape
end

5.times { FiveShapes.new.create_random_ivars }
15.times { TooComplex.new.create_random_ivars }

# Create an instance with instance variable foo. The benchmark will access it repeatedly
single = SingleShape.new(:@foo)
five = FiveShapes.new(:@foo)
too_complex = TooComplex.new(:@foo)

Benchmark.ips do |x|
  x.report('1') { single.access }
  x.report('5') { five.access }
  x.report('8+') { too_complex.access }
  x.compare!
end

Only one class gets the performance penalty:

test.rb:20: warning: The class TooComplex reached 8 shape variations, instance variables accesses will be slower and memory usage increased.
It is recommended to define instance variables in a consistent order, for instance by eagerly defining them all in the #initialize method.
ruby 3.4.3 (2025-04-14 revision d0b7e5b6a0) +PRISM [x86_64-linux]
Warming up --------------------------------------
                   1     2.227M i/100ms
                   5     2.218M i/100ms
                  8+     1.947M i/100ms
Calculating -------------------------------------
                   1     21.923M (± 0.9%) i/s   (45.61 ns/i) -    111.354M in   5.079777s
                   5     21.949M (± 0.7%) i/s   (45.56 ns/i) -    110.918M in   5.053766s
                  8+     19.113M (± 1.8%) i/s   (52.32 ns/i) -     97.366M in   5.095955s

Comparison:
                   5: 21948791.2 i/s
                   1: 21923011.1 i/s - same-ish: difference falls within error
                  8+: 19113198.0 i/s - 1.15x  slower

With JIT enabled, the penality is a bit higher at ~1.45x slower, so that benefits more from fewer shapes.

I think the warning from Ruby with Warning[:performance] = true (you have to explicitly enable those, -W doesn't cut it) is a better solution. It also considers all the funny ways you can set ivars via metaprogramming (or simply in another file) that rubocop has no chance of knowing about.

Earlopain avatar Aug 29 '25 15:08 Earlopain

Warning[:performance] = true

TIL https://alchemists.io/articles/ruby_warnings

schneems avatar Aug 29 '25 15:08 schneems