When calling a method that accepts an anonymous splat and literal
keywords without any arguments, an assertion failure was previously
raised. Set rest_index to 0 when setting rest to the frozen hash,
so the args_argc calculation is accurate.
While here, add more tests for methods with anonymous splats with
and without keywords and keyword splats to confirm behavior is
correct.
Also add a basic bootstrap test that would hit the previous assertion
failure.
Co-authored-by: Jean Boussier <jean.boussier@gmail.com>
When we forward an FCALL (a method call with an implicit self), we
shouldn't forward the FCALL flag because it ignores method visibility
checks. This patch removes the FCALL flag from callers.
[Bug #21196]
I don't think we should ever consider forwarded IC's to be "simple".
Previously, the "simple" flag would be copied to the derived IC and this
happened to cause struct set / get iseqs to write an invalid CC
fastpath:
f45eb3dcb9/vm_insnhelper.c (L4726-L4729)
[Bug #20799]
When we forward calls to C functions if the callsite is a forwarding
site it might not always be a splat, so we can't use the fast path.
Fixes:
[ruby-core:118418]
combination with `send` method (optimized) or `method_missing`
and forwarding send (`...`) needs to respect given
`rb_forwarding_call_data`. Otherwize it causes critical error
such as SEGV.
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>
Previously on builds with optimizations disabled, this could result in
an out of bounds read. When we had all of:
* built with -O0
* Leaf builtin
* Primitive.mandatory_only
* "no args builtin", called by vm_call_single_noarg_inline_builti
* The stack is escaped to the heap via binding or a proc
This is because mk_builtin_loader generated reads for all locals
regardless of whether they were used and in the case we generated a
mandatory_only iseq that would include more variables than were actually
available.
On optimized builds, the invalid accesses would be optimized away, and
this also was often unnoticed as the invalid access would just hit
another part of the stack unless it had been escaped to the heap.
The fix here is imperfect, as this could have false positives, but since
Primitive.cexpr! is only available within the cruby codebase itself
that's probably fine as a proper fix would be much more challenging (the
only false positives we found were in rjit.rb).
Fixes [Bug #20178]
Co-authored-by: Adam Hess <HParker@github.com>
* `-j` option for concurrent test with threads
* `-jN` uses N threads
* `-j` uses nproc/2 threads
* Introduce `BT` struct to manage configurations
* Introduce `Assertion` to manage all assertions
* Remove all toplevel instance variables
* Show elapsed seconds at last
```
$ time make btest
...
real 0m37.319s
user 0m26.221s
sys 0m16.534s
$ time make btest TESTOPTS=-j
...
real 0m11.812s
user 0m36.667s
sys 0m21.872s
```
remove tests for $SAFE=4.
* lib/pp.rb: use taint instead of untrust to avoid warnings when
$VERBOSE is set to true.
git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@41269 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
ci->argc and ci->blockptr before and after method invocations
because these method dispatches override call_info.
* bootstraptest/test_method.rb: add tests for this fix.
git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@37641 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
this method call needs splat argument because cahced functions
(vm_call_attrset, vm_call_ivar, vm_call_cfunc_fast_(unary|binary))
do not check an arity.
* bootstraptest/test_method.rb: add a test to check an above issue.
git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@37199 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
added. Unlinked method entries are collected to
vm->unlinked_method_entry_list. On the GC timing, mark all method
entries which are on all living threads. Only non-marked method
entries are collected. This hack prevents releasing living method
entry.
[Performance Consideration] Since this Method Entry GC (MEGC)
doesn't occuer frequently, MEGC will not be a performance bottleneck.
However, to traverse living method entries, every control frame push
needs to clear cfp->me field. This will be a performance issue
(because pushing control frame is occurred frequently).
Bug #2777 [ruby-dev:40457]
* cont.c (fiber_init): init cfp->me.
* gc.c (garbage_collect): kick rb_sweep_method_entry().
* method.h (rb_method_entry_t): add a mark field.
* vm.c (invoke_block_from_c): set passed me.
* vm.c (rb_thread_mark): mark cfp->me.
* vm_core.h (rb_thread_t): add a field passed_me.
* vm_core.h (rb_vm_t): add a field unlinked_method_entry_list.
* vm_insnhelper.c (vm_push_frame): clear cfp->me at all times.
* vm_insnhelper.c (vm_call_bmethod): pass me.
* bootstraptest/test_method.rb: add a test.
git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@27634 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
is method_missing. [ruby-core:22298]
* vm_eval.c (rb_raise_method_missing): new function to directly
raise NoMethodError.
* vm_insnhelper.c (vm_call_method): fixed the case method_missing
is missing.
git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@22495 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
it now invokes public method only no matter how it's called.
git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@14173 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
visibility again. we no longer provide __send, __send!.
* eval.c (rb_invoke_method): new method to honor private
visibility. if it's invoked in a function call style, it calls
private methods as well (previous 1.9 send behavior).
git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@13824 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
if x= is private.
* bootstraptest/test_method.rb: add a test for above.
git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@13093 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
insnhelper.ci (vm_call_method): fix to save safelevel for
method node.
* include/ruby/node.h: ditto.
* bootstraptest/test_method.rb: add a test for above.
git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@13079 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
[ruby-dev:31110]
* bootstraptest/test_method.rb: add a test for above.
git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@12686 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
[ruby-dev:31094]
* bootstraptest/test_method.rb: add a test for above.
git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@12684 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
