Could we simplify OpenStruct to not define any methods, just method_missing + Hash and < BasicObject?

[eregon]

See https://bugs.ruby-lang.org/issues/19424#note-11
The idea comes from oracle/truffleruby#2702 (comment)
I think this would be a nice simplification, and would be more intuitive performance-wise.

Defining methods dynamically is extremely expensive (also even just forcing a singleton class is expensive as well), even more so on more optimizing Ruby implementations.
Slightly slower for all access but no hidden costs on defining new members/new instance seems better.

Any thought on that?
I should be able to provide a POC PR, just asking if there is known compatibility issue with that approach.

[eregon]

Inheriting from BasicObject would allow "overriding" Object methods by simply not having them, but that's probably too incompatible as it means Kernel's public instance methods wouldn't be defined on OpenStruct like class respond_to? is_a? object_id, only the !-suffix copies.
EDIT: Unless we in method_missing we support delegating to Kernel methods.
Another incompatibility would be constant lookup would need e.g. ::String instead of String in OpenStruct subclasses, but that may matter little.

If we inherit from Object then it doesn't allow overriding them anymore, because method_missing is of course only called if the method doesn't already exists.

[eregon]

I tried it as POC changes, and with those changes the tests pass.

Here it is with OpenStruct < Object, which doesn't handle overriding Kernel methods:
master...eregon:ostruct:simplify-inherit-Object-not-overridable

And here it is with OpenStruct < BasicObject, which is more compatible (handles overriding Kernel methods but not overriding OpenStruct methods) and delegates to Kernel in method_missing:
master...eregon:ostruct:simplify-basic-object

Probably the most interesting parts is the changes to test/ostruct/test_ostruct.rb to see how compatible they are.

[marcandre]

Thanks for raising this issue.

Personally, I am completey un-sympathetic to performance issues about OpenStruct, in particular about un-marshalling OpenStructs!

I am also un-sympathetic to simplifying the code at the expense of compatibility and functionality.

[eregon]

I benchmarked master...eregon:ostruct:simplify-basic-object, since that's more compatible and probably could be made 100% compatible easily (by moving OpenStruct methods to some Procs or explicit handling in method_missing so).

The main point of this change is performance, but also IMO it simplifies the code and logic significantly.
Here is my benchmark (EDIT: I changed to the updated benchmark which forces the allocation of the object to be more representative):

puts RUBY_DESCRIPTION
require 'benchmark/ips'
require 'ostruct'

class MyClass
  attr_accessor :a, :b, :c
  
  def initialize(a:, b:, c:)
    @a = a
    @b = b
    @c = c
  end
end

Benchmark.ips do |x|
  obj = nil
  result = 0
  
  x.report("OpenStruct") do
    obj = OpenStruct.new(a: 4, b: 7, c: 9)
    # obj.a = 4
    # obj.b = 7
    # obj.c = 9
    result = obj.a * obj.b * obj.c
  end
  
  x.report("MyClass") do
    obj = MyClass.new(a: 4, b: 7, c: 9)
    result = obj.a * obj.b * obj.c
  end

  x.compare!
end

In (iterations per second, times slower than Class):

Ruby	Class	current OpenStruct	OpenStruct my branch	Speedup my branch/current
3.2.0	2.079M	68.471k, 30.37x	438.532k, 4.58x	6.6x
3.2.0 YJIT	2.582M	9.242k, 279.36x	563.449k, 4.51x	61.9x
JRuby 9.4.0	3.018M	167.715k, 18.00x	1.488M, 1.96x	9.2x
TruffleRuby Native	106.094M	14.629k, 7252.16x	30.141M, 3.46x	2096x

Here is the full output: https://gist.github.com/eregon/b65909bd1b9a7c45dc1e3c58df194e85?permalink_comment_id=4496630#gistcomment-4496630

Which makes my point about OpenStruct being horribly expensive, and "even more so on more optimizing Ruby implementations." very clear.

We need to do something here, the performance of OpenStruct is too terrible, especially on faster Rubies.
The only alternative I see would be to deprecate the OpenStruct constant and stop shipping it with Ruby.

Also https://twitter.com/coolprobn/status/1632243667531624448

[eregon]

I updated the benchmark to force the allocation, using the variant in https://gist.github.com/eregon/b65909bd1b9a7c45dc1e3c58df194e85?permalink_comment_id=4496630#gistcomment-4496630.
The results are basically the same except on TruffleRuby, which optimized away the previous benchmark as it didn't force the allocation. Now the TruffleRuby numbers are more representative of real usages of OpenStruct.

[eregon]

One thing that came up discussing this on the CRuby Slack is inheriting from BasicObject has the problem that methods defined in subclasses of OpenStruct would need e.g ::String and not just String.
Example of OpenStruct subclasses:

• https://github.com/aetherknight/recursive-open-struct/blob/main/lib/recursive_open_struct.rb#L15
• https://github.com/collectiveidea/interactor/blob/master/lib/interactor/context.rb#L31
• https://github.com/stimulusreflex/stimulus_reflex/blob/5b64860eeab64b4f11614251bd3e2df456c04f60/lib/stimulus_reflex/dataset.rb#L5

So probably we shouldn't use BasicObject but make our own "BlankSlate".
Not sure this strange property of BasicObject and subclasses is ever useful (to not look into Object constants), maybe we should change that in Ruby, but that won't help us short term.