This commit ensures warnings about `object_id` and `__send__` method
redefinitions are emitted for other method types such as aliases, procs,
and attr readers—anything except C functions.
`me->defined_class` will be used for ancestor searching
so that it should be T_CLASS or T_ICLASS.
This patch will resoleve https://github.com/ruby/ruby/pull/11715
This patch optimizes forwarding callers and callees. It only optimizes methods that only take `...` as their parameter, and then pass `...` to other calls.
Calls it optimizes look like this:
```ruby
def bar(a) = a
def foo(...) = bar(...) # optimized
foo(123)
```
```ruby
def bar(a) = a
def foo(...) = bar(1, 2, ...) # optimized
foo(123)
```
```ruby
def bar(*a) = a
def foo(...)
list = [1, 2]
bar(*list, ...) # optimized
end
foo(123)
```
All variants of the above but using `super` are also optimized, including a bare super like this:
```ruby
def foo(...)
super
end
```
This patch eliminates intermediate allocations made when calling methods that accept `...`.
We can observe allocation elimination like this:
```ruby
def m
x = GC.stat(:total_allocated_objects)
yield
GC.stat(:total_allocated_objects) - x
end
def bar(a) = a
def foo(...) = bar(...)
def test
m { foo(123) }
end
test
p test # allocates 1 object on master, but 0 objects with this patch
```
```ruby
def bar(a, b:) = a + b
def foo(...) = bar(...)
def test
m { foo(1, b: 2) }
end
test
p test # allocates 2 objects on master, but 0 objects with this patch
```
How does it work?
-----------------
This patch works by using a dynamic stack size when passing forwarded parameters to callees.
The caller's info object (known as the "CI") contains the stack size of the
parameters, so we pass the CI object itself as a parameter to the callee.
When forwarding parameters, the forwarding ISeq uses the caller's CI to determine how much stack to copy, then copies the caller's stack before calling the callee.
The CI at the forwarded call site is adjusted using information from the caller's CI.
I think this description is kind of confusing, so let's walk through an example with code.
```ruby
def delegatee(a, b) = a + b
def delegator(...)
delegatee(...) # CI2 (FORWARDING)
end
def caller
delegator(1, 2) # CI1 (argc: 2)
end
```
Before we call the delegator method, the stack looks like this:
```
Executing Line | Code | Stack
---------------+---------------------------------------+--------
1| def delegatee(a, b) = a + b | self
2| | 1
3| def delegator(...) | 2
4| # |
5| delegatee(...) # CI2 (FORWARDING) |
6| end |
7| |
8| def caller |
-> 9| delegator(1, 2) # CI1 (argc: 2) |
10| end |
```
The ISeq for `delegator` is tagged as "forwardable", so when `caller` calls in
to `delegator`, it writes `CI1` on to the stack as a local variable for the
`delegator` method. The `delegator` method has a special local called `...`
that holds the caller's CI object.
Here is the ISeq disasm fo `delegator`:
```
== disasm: #<ISeq:delegator@-e:1 (1,0)-(1,39)>
local table (size: 1, argc: 0 [opts: 0, rest: -1, post: 0, block: -1, kw: -1@-1, kwrest: -1])
[ 1] "..."@0
0000 putself ( 1)[LiCa]
0001 getlocal_WC_0 "..."@0
0003 send <calldata!mid:delegatee, argc:0, FCALL|FORWARDING>, nil
0006 leave [Re]
```
The local called `...` will contain the caller's CI: CI1.
Here is the stack when we enter `delegator`:
```
Executing Line | Code | Stack
---------------+---------------------------------------+--------
1| def delegatee(a, b) = a + b | self
2| | 1
3| def delegator(...) | 2
-> 4| # | CI1 (argc: 2)
5| delegatee(...) # CI2 (FORWARDING) | cref_or_me
6| end | specval
7| | type
8| def caller |
9| delegator(1, 2) # CI1 (argc: 2) |
10| end |
```
The CI at `delegatee` on line 5 is tagged as "FORWARDING", so it knows to
memcopy the caller's stack before calling `delegatee`. In this case, it will
memcopy self, 1, and 2 to the stack before calling `delegatee`. It knows how much
memory to copy from the caller because `CI1` contains stack size information
(argc: 2).
Before executing the `send` instruction, we push `...` on the stack. The
`send` instruction pops `...`, and because it is tagged with `FORWARDING`, it
knows to memcopy (using the information in the CI it just popped):
```
== disasm: #<ISeq:delegator@-e:1 (1,0)-(1,39)>
local table (size: 1, argc: 0 [opts: 0, rest: -1, post: 0, block: -1, kw: -1@-1, kwrest: -1])
[ 1] "..."@0
0000 putself ( 1)[LiCa]
0001 getlocal_WC_0 "..."@0
0003 send <calldata!mid:delegatee, argc:0, FCALL|FORWARDING>, nil
0006 leave [Re]
```
Instruction 001 puts the caller's CI on the stack. `send` is tagged with
FORWARDING, so it reads the CI and _copies_ the callers stack to this stack:
```
Executing Line | Code | Stack
---------------+---------------------------------------+--------
1| def delegatee(a, b) = a + b | self
2| | 1
3| def delegator(...) | 2
4| # | CI1 (argc: 2)
-> 5| delegatee(...) # CI2 (FORWARDING) | cref_or_me
6| end | specval
7| | type
8| def caller | self
9| delegator(1, 2) # CI1 (argc: 2) | 1
10| end | 2
```
The "FORWARDING" call site combines information from CI1 with CI2 in order
to support passing other values in addition to the `...` value, as well as
perfectly forward splat args, kwargs, etc.
Since we're able to copy the stack from `caller` in to `delegator`'s stack, we
can avoid allocating objects.
I want to do this to eliminate object allocations for delegate methods.
My long term goal is to implement `Class#new` in Ruby and it uses `...`.
I was able to implement `Class#new` in Ruby
[here](https://github.com/ruby/ruby/pull/9289).
If we adopt the technique in this patch, then we can optimize allocating
objects that take keyword parameters for `initialize`.
For example, this code will allocate 2 objects: one for `SomeObject`, and one
for the kwargs:
```ruby
SomeObject.new(foo: 1)
```
If we combine this technique, plus implement `Class#new` in Ruby, then we can
reduce allocations for this common operation.
Co-Authored-By: John Hawthorn <john@hawthorn.email>
Co-Authored-By: Alan Wu <XrXr@users.noreply.github.com>
Currently redefining a method doesn't emit one but two warnings.
One at the location of the new method, and one at the location of
the old method.
I believe this is not ideal because when collecting warnings via
a custom `Warning.warn`, it has to be pieced together. It's even
more tricky because the second part may or may not be emitted
depending on whether the original method has an associated ISeq.
I think it's much better to emit a single warning with all the
information in one go.
When we're searching for CCs, compare the argc and flags for CI rather
than comparing pointers. This means we don't need to store a reference
to the CI, and it also naturally "de-duplicates" CC objects.
We can observe the effect with the following code:
```ruby
require "objspace"
hash = {}
p ObjectSpace.memsize_of(Hash)
eval ("a".."zzz").map { |key|
"hash.merge(:#{key} => 1)"
}.join("; ")
p ObjectSpace.memsize_of(Hash)
```
On master:
```
$ ruby -v test.rb
ruby 3.4.0dev (2024-04-15T16:21:41Z master d019b3baec) [arm64-darwin23]
test.rb:3: warning: assigned but unused variable - hash
3424
527736
```
On this branch:
```
$ make runruby
compiling vm.c
linking miniruby
builtin_binary.inc updated
compiling builtin.c
linking static-library libruby.3.4-static.a
ln -sf ../../rbconfig.rb .ext/arm64-darwin23/rbconfig.rb
linking ruby
ld: warning: ignoring duplicate libraries: '-ldl', '-lobjc', '-lpthread'
RUBY_ON_BUG='gdb -x ./.gdbinit -p' ./miniruby -I./lib -I. -I.ext/common ./tool/runruby.rb --extout=.ext -- --disable-gems ./test.rb
2240
2368
```
Co-authored-by: John Hawthorn <jhawthorn@github.com>
In cfd7729ce7 we started using inline
caches for refinements. However, we weren't clearing inline caches when
defined on a reopened refinement module.
Fixes [Bug #20246]
This frees FL_USER0 on both T_MODULE and T_CLASS.
Note: prior to this, FL_SINGLETON was never set on T_MODULE,
so checking for `FL_SINGLETON` without first checking that
`FL_TYPE` was `T_CLASS` was valid. That's no longer the case.
Rather than exposing that an imemo has a flag and four fields, this
changes the implementation to only expose one field (the klass) and
fills the rest with 0. The type will have to fill in the values themselves.
Previously every call to vm_ci_new (when the CI was not packable) would
result in a different callinfo being returned this meant that every
kwarg callsite had its own CI.
When calling, different CIs result in different CCs. These CIs and CCs
both end up persisted on the T_CLASS inside cc_tbl. So in an eval loop
this resulted in a memory leak of both types of object. This also likely
resulted in extra memory used, and extra time searching, in non-eval
cases.
For simplicity in this commit I always allocate a CI object inside
rb_vm_ci_lookup, but ideally we would lazily allocate it only when
needed. I hope to do that as a follow up in the future.
Previously, we didn't invalidate the method entry wrapped by
VM_METHOD_TYPE_REFINED method entries which could cause calls to
land in the wrong method like it did in the included test.
Do the invalidation, and adjust rb_method_entry_clone() to accommodate
this new invalidation vector.
Fix: cfd7729ce7
See-also: e201b81f79
Found through GC.stress + GC.auto_compact crashes in GH-8932.
Previously, the compaction run within `rb_method_entry_alloc()` could
move the `def->body.iseq.cref` and `iseqptr` set up before the call and
leave the `def` pointing to moved addresses. Nothing was marking `def`
during that GC run.
Low probability reproducer:
GC.stress = true
GC.auto_compact = true
arr = []
alloc = 1000.times.map { [] }
alloc = nil
a = arr.first
GC.start
fix memory leak in vm_method
This introduces a unified reference_count to clarify who is referencing a method.
This also allows us to treat the refinement method as the def owner since it counts itself as a reference
Co-authored-by: Peter Zhu <peter@peterzhu.ca>
[Bug #19894]
When a copy of a complemented method entry is created, there are two
issues:
1. IMEMO_FL_USER3 is not copied, so the complemented status is not
copied over.
2. In rb_method_entry_clone we increment both alias_count and
complemented_count. However, when we free the method entry in
rb_method_definition_release, we only decrement one of the two
counters, resulting in the rb_method_definition_t being leaked.
Co-authored-by: Adam Hess <adamhess1991@gmail.com>
From Ruby 3.0, refined method invocations are slow because
resolved methods are not cached by inline cache because of
conservertive strategy. However, `using` clears all caches
so that it seems safe to cache resolved method entries.
This patch caches resolved method entries in inline cache
and clear all of inline method caches when `using` is called.
fix [Bug #18572]
```ruby
# without refinements
class C
def foo = :C
end
N = 1_000_000
obj = C.new
require 'benchmark'
Benchmark.bm{|x|
x.report{N.times{
obj.foo; obj.foo; obj.foo; obj.foo; obj.foo;
obj.foo; obj.foo; obj.foo; obj.foo; obj.foo;
obj.foo; obj.foo; obj.foo; obj.foo; obj.foo;
obj.foo; obj.foo; obj.foo; obj.foo; obj.foo;
}}
}
_END__
user system total real
master 0.362859 0.002544 0.365403 ( 0.365424)
modified 0.357251 0.000000 0.357251 ( 0.357258)
```
```ruby
# with refinment but without using
class C
def foo = :C
end
module R
refine C do
def foo = :R
end
end
N = 1_000_000
obj = C.new
require 'benchmark'
Benchmark.bm{|x|
x.report{N.times{
obj.foo; obj.foo; obj.foo; obj.foo; obj.foo;
obj.foo; obj.foo; obj.foo; obj.foo; obj.foo;
obj.foo; obj.foo; obj.foo; obj.foo; obj.foo;
obj.foo; obj.foo; obj.foo; obj.foo; obj.foo;
}}
}
__END__
user system total real
master 0.957182 0.000000 0.957182 ( 0.957212)
modified 0.359228 0.000000 0.359228 ( 0.359238)
```
```ruby
# with using
class C
def foo = :C
end
module R
refine C do
def foo = :R
end
end
N = 1_000_000
using R
obj = C.new
require 'benchmark'
Benchmark.bm{|x|
x.report{N.times{
obj.foo; obj.foo; obj.foo; obj.foo; obj.foo;
obj.foo; obj.foo; obj.foo; obj.foo; obj.foo;
obj.foo; obj.foo; obj.foo; obj.foo; obj.foo;
obj.foo; obj.foo; obj.foo; obj.foo; obj.foo;
}}
}
`struct rb_calling_info::cd` is introduced and `rb_calling_info::ci`
is replaced with it to manipulate the inline cache of iseq while
method invocation process. So that `ci` can be acessed with
`calling->cd->ci`. It adds one indirection but it can be justified
by the following points:
1) `vm_search_method_fastpath()` doesn't need `ci` and also
`vm_call_iseq_setup_normal()` doesn't need `ci`. It means
reducing `cd->ci` access in `vm_sendish()` can make it faster.
2) most of method types need to access `ci` once in theory
so that 1 additional indirection doesn't matter.
Given that signleton classes don't have an allocator,
we can re-use these bytes to store the attached object
in `rb_classext_struct` without making it larger.
Right now the attached object is stored as an instance variable
and all the call sites that either get or set it have to know how it's
stored.
It's preferable to hide this implementation detail behind accessors
so that it is easier to change how it's stored.
Previously, the following crashes with
`CFLAGS=-DRGENGC_CHECK_MODE=2 -DRUBY_DEBUG=1 -fno-inline`:
$ ./miniruby -e 'GC.stress = true; Marshal.dump({})'
It crashes with a write barrier (WB) miss assertion on an edge from the
`Hash` class object to a newly allocated negative method entry.
This is due to usages of vm_ccs_create() running the WB too early,
before the method entry is inserted into the cc table, so before the
reference edge is established. The insertion can trigger GC and promote
the class object, so running the WB after the insertion is necessary.
Move the insertion into vm_ccs_create() and run the WB after the
insertion.
Discovered on CI:
http://ci.rvm.jp/results/trunk-asserts@ruby-sp2-docker/4391770
I noticed this while running test_yjit with --mjit-call-threshold=1,
which redefines `Integer#<`. When Ruby is monkey-patched,
MJIT itself could be broken.
Similarly, Ruby scripts could break MJIT in many different ways. I
prepared the same set of hooks as YJIT so that we could possibly
override it and disable it on those moments. Every constant under
RubyVM::MJIT is private and thus it's an unsupported behavior though.
Previously, the frozen check happened on `RCLASS_ORIGIN(self)`, which
can return an iclass. The frozen check is supposed to respond to objects
that users can call methods on while iclasses are hidden from users.
Other mutation methods like Module#{define_method,alias_method,public}
don't do this. Check frozen status on the module itself.
Fixes [Bug #19164] and [Bug #19166].
Co-authored-by: Alan Wu <XrXr@users.noreply.github.com>
Other functions are already type-punned elsewhere. rb_f_notimplement is
the only exceptional function that appear literally. We have to take
care of it by hand.
df317151a5 removed the code to free
rb_hook_list_t, so repeated targeting of the same bmethod started
to leak the hook list. You can observe how the maximum memory use
scales with input size in the following script with `/usr/bin/time -v`.
```ruby
o = Object.new
o.define_singleton_method(:foo) {}
trace = TracePoint.new(:return) {}
bmethod = o.method(:foo)
ARGV.first.to_i.times { trace.enable(target:bmethod){} }
4.times {GC.start}
```
After this change the maximum doesn't grow as quickly.
To plug the leak, check whether the hook list is already allocated
when enabling the targeting TracePoint for the bmethod. This fix
also allows multiple TracePoints to target the same bmethod, similar
to other valid TracePoint targets.
Finally, free the rb_hook_list_t struct when freeing the method
definition it lives on. Freeing in the GC is a good way to avoid
lifetime problems similar to the one fixed in df31715.
[Bug #18031]
Nobuyoshi Nakada
Thomas Marshall
Koichi Sasada
Peter Zhu
Aaron Patterson
Jeremy Evans
Jean Boussier
John Hawthorn
Yusuke Endoh
Takashi Kokubun
Alan Wu
Adam Hess
Jean byroot Boussier
S-H-GAMELINKS
Samuel Williams
卜部昌平