ostruct icon indicating copy to clipboard operation
ostruct copied to clipboard

Use a generic getter with __callee__

Open headius opened this issue 2 years ago • 3 comments

This modifies the getter definition to use a single method that uses __callee__ to access the hash.

def __get_callee__!
  @table[__callee__]
end

On versions of Ruby 2.0+ and JRuby 9.4.2+ __callee__ will return the aliased name, which in this case is the key for the table.

This eliminates one of the Proc-based methods per field and also performs better on CRuby:

Before:

Warming up --------------------------------------
                 get    44.000  i/100ms
                 set    46.000  i/100ms
Calculating -------------------------------------
                 get    504.731  (± 8.5%) i/s -      2.508k in   5.024439s
                 set    506.779  (± 1.4%) i/s -      2.576k in   5.084061s

After:

Warming up --------------------------------------
                 get    80.000  i/100ms
                 set    65.000  i/100ms
Calculating -------------------------------------
                 get    808.319  (± 0.7%) i/s -      4.080k in   5.047805s
                 set    662.678  (± 0.9%) i/s -      3.315k in   5.002857s

It also uses less memory; creating 100k OpenStruct in the same way as the benchmark uses 205.2MB before and 164MB after.

Note that a similar memory savings is possible by using the following setter:

def __set_callee__!(value)
  @name[__callee__[0..-2].intern] = value
end

But it is quite a bit slower than the Proc version. I think it would be worth exploring other ways we can simplify the setters too.

headius avatar Aug 09 '23 17:08 headius

Relates to https://github.com/jruby/jruby/issues/7876 where we noticed ostruct uses a LOT more memory now that it pre-creates all singleton methods in response to the hash form of OpenStruct#new.

headius avatar Aug 09 '23 17:08 headius

Note: I also tried using class_eval to create these methods with literal symbols. That was easily the fastest-performing mechanism for getter/setter methods, but it also consumed even more memory than the Proc version. 🤷‍♂️

headius avatar Aug 09 '23 17:08 headius

FWIW I know this library is not intended for performance-sensitive applications, but I think the speed and memory improvements here are worth the small change.

headius avatar Aug 09 '23 18:08 headius