[ruby-core:106193] [Ruby master Misc#18354] Lazily create singletons on instance_{exec, eval}

Issue #18354 has been reported by jhawthorn (John Hawthorn).

----------------------------------------
Misc #18354: Lazily create singletons on instance_{exec,eval}
https://bugs.ruby-lang.org/issues/18354

* Author: jhawthorn (John Hawthorn)
* Status: Open
* Priority: Normal
----------------------------------------
GitHub PR: https://github.com/ruby/ruby/pull/5146

Previously when `instance_exec` or `instance_eval` was called on an object, that object would be given a singleton class so that method definitions inside the block would be added to the object rather than its class. There was some discussion of this in #18276.

This commit aims to improve performance by delaying the creation of the singleton class unless one is needed for method definition. Most of the time `instance_eval` is used without any method definition.

This change makes the RailsBench benchmark from yjit-bench **1.09x faster for the interpreter** and makes **YJIT 1.16x faster when enabled.**

This was implemented by adding a flag to the cref indicating that it represents a singleton of the object rather than a class itself. In this case `CREF_CLASS` returns the object's existing class, but in cases that we are defining a method (either via `definemethod` or `VM_SPECIAL_OBJECT_CBASE`/`VM_SPECIAL_OBJECT_CONST_BASE` which is used for undef, alias, and constant definitions).

This also happens to fix what I [believe](https://twitter.com/jhawthorn/status/1460442596417826821) is a bug. Previously `instance_eval` behaved differently with regards to constant access for `true`/`false`/`nil` than for all other objects. I don't think this was intentional.

    String::Foo = "foo"
    "".instance_eval("Foo")   # => "foo"
    Integer::Foo = "foo"
    123.instance_eval("Foo")  # => "foo"
    TrueClass::Foo = "foo"
    true.instance_eval("Foo") # NameError: uninitialized constant Foo

With this change TrueClass/NilClass/FalseClass behave the same as everything else.

This also slightly changes the error message when trying to define a method  through `instance_eval` on an object which can't have a singleton class.

Before:

    $ ruby -e '123.instance_eval { def foo; end }'
    -e:1:in `block in <main>': no class/module to add method (TypeError)

After:

    $ ./ruby -e '123.instance_eval { def foo; end }'
    -e:1:in `block in <main>': can't define singleton (TypeError)

IMO this error is a small improvement on the original and better matches
the (both old and new) message when definging a method using `def self.`

    $ ruby -e '123.instance_eval{ def self.foo; end }'
    -e:1:in `block in <main>': can't define singleton (TypeError)

---

With this change we can observe that instance_eval doesn't change an object's class unless necessary.

**Before**

```
$ ruby -robjspace -e 'obj = Object.new; puts ObjectSpace.dump(obj); obj.instance_eval { self }; puts ObjectSpace.dump(obj)'
{"address":"0x562155967e00", "type":"OBJECT", "class":"0x56215571a8a0", "ivars":3, "memsize":40, "flags":{"wb_protected":true}}
{"address":"0x562155967e00", "type":"OBJECT", "class":"0x562155967ae0", "ivars":3, "memsize":40, "flags":{"wb_protected":true}}
```

(the "class" address changes)

**After**

```
$ ./ruby -robjspace -e 'obj = Object.new; puts ObjectSpace.dump(obj); obj.instance_eval { self }; puts ObjectSpace.dump(obj)'
{"address":"0x7fa089ce7698", "type":"OBJECT", "class":"0x7fa08d19e850", "ivars":3, "memsize":40, "flags":{"wb_protected":true}}
{"address":"0x7fa089ce7698", "type":"OBJECT", "class":"0x7fa08d19e850", "ivars":3, "memsize":40, "flags":{"wb_protected":true}}
```

(the "class" address remains the same)

---

This should be particularly helpful for Rails apps, which use `instance_eval` as part of the `ActiveSupport::Callbacks` mechanism when provided a `Proc` (which is common for developers to do). Under the interpreter, this should be faster due to not allocating a new singleton, and keeping method entries and inline caches valid. Under both MJIT and YJIT this should be even more helpful as we'll be able to use jitted methods on objects which previously had been given singleton classes.

I ran railsbench from yjit-bench on this (on my local AMD zen2 Linux machine) and the numbers look great.

**Before**

```
end_time="2021-11-18 15:57:24 PST (-0800)"
yjit_opts=""
ruby_version="ruby 3.1.0dev (2021-11-18T23:49:36Z lazy_singleton 8ba9639805) [x86_64-linux]"
git_branch="lazy_singleton"
git_commit="8ba9639805"

----------  -----------  ----------  ---------  ----------  -----------  ------------
bench       interp (ms)  stddev (%)  yjit (ms)  stddev (%)  interp/yjit  yjit 1st itr
railsbench  2092.0       1.0         1644.6     1.8         1.27         1.24
----------  -----------  ----------  ---------  ----------  -----------  ------------
Legend:
- interp/yjit: ratio of interp/yjit time. Higher is better. Above 1 represents a speedup.
- 1st itr: ratio of interp/yjit time for the first benchmarking iteration.
```

**After**

```
end_time="2021-11-18 16:03:25 PST (-0800)"
yjit_opts=""
ruby_version="ruby 3.1.0dev (2021-11-18T21:57:23Z lazy_singleton f09b438e6b) [x86_64-linux]"
git_branch="lazy_singleton"
git_commit="f09b438e6b"

----------  -----------  ----------  ---------  ----------  -----------  ------------
bench       interp (ms)  stddev (%)  yjit (ms)  stddev (%)  interp/yjit  yjit 1st itr
railsbench  1908.1       1.1         1415.9     1.6         1.35         1.29
----------  -----------  ----------  ---------  ----------  -----------  ------------
Legend:
- interp/yjit: ratio of interp/yjit time. Higher is better. Above 1 represents a speedup.
- 1st itr: ratio of interp/yjit time for the first benchmarking iteration.
```

So this change makes YJIT 1.16x faster than it was previously, and the interpreter 1.09x faster than it used to be! (and for fun old_interp/new_yjit = 1.47)

I also made an exaggerated benchmark to show the effect of this on a standalone Rails app. https://gist.github.com/jhawthorn/42559732de3c5755ba1f3f6e2796536c This shows how previously just adding even a single empty callback (which uses instance_exec) significantly impacts performance for the rest of the request.

---Files--------------------------------
0001-Lazily-create-singletons-on-instance_-exec-eval.patch (17.5 KB)


-- 
https://bugs.ruby-lang.org/

Unsubscribe: <mailto:[e-mail]?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>