| Age | Commit message (Collapse) | Author |
|---|
|
|
|
Notes:
Merged: https://github.com/ruby/ruby/pull/11501
|
|
Apply similar fix to https://github.com/ruby/prism/pull/2944
Notes:
Merged: https://github.com/ruby/ruby/pull/11500
|
|
Notes:
Merged: https://github.com/ruby/ruby/pull/11482
|
|
Notes:
Merged: https://github.com/ruby/ruby/pull/11496
|
|
Notes:
Merged: https://github.com/ruby/ruby/pull/11489
|
|
Match logic in compile.c:2133. Without this, the unused block warning
code allocates an array, causing TestAllocation to fail.
Notes:
Merged: https://github.com/ruby/ruby/pull/11484
|
|
Rename some fields that do not quite make sense.
* CaseMatchNode#consequent -> CaseMatchNode#else_clause
* CaseNode#consequent -> CaseNode#else_clause
* IfNode#consequent -> IfNode#subsequent
* RescueNode#consequent -> RescueNode#subsequent
* UnlessNode#consequent -> UnlessNode#else_clause
Notes:
Merged: https://github.com/ruby/ruby/pull/11480
|
|
Part of implementing 3de20efc308cccc38bf9dacfffca6c626d039a06 in
prism_compile.c. Down to 2 failures from 30 failures in
test/ruby/test_allocation.rb.
Notes:
Merged: https://github.com/ruby/ruby/pull/11479
|
|
With the parse.y parser, when a fifo (named pipe) is passed to
Kernel#load and friends, we wait for data to be available first before
reading. Note that with fifos, opening with `O_RDONLY|O_NONBLOCK` and
then reading will look like EOF with read(2) returning 0, but data can
become available later.
The prism compiler needs to match this behavior to pass
`test_loading_fifo_{fd_leak,threading_raise,threading_success}`. I chose
to use IO#read to do this.
An alternative way to match behavior would be to use open_load_file()
from ruby.c like parse.y, but I opted to only allocate an IO to deal
with threading when reading from pipes and character devices. The
memory mapping code seems to work fine for regular files.
Notes:
Merged: https://github.com/ruby/ruby/pull/11472
|
|
Fixes the following allocations tests:
* `test_keyword_and_keyword_splat_parameter`
* `test_keyword_parameter`
* `test_keyword_splat_parameter`
* `test_no_array_allocation_with_splat_and_nonstatic_keywords`
* `test_no_parameters`
* `test_positional_splat_and_keyword_splat_parameter`
* `test_ruby2_keywords`
* Checks for `first_chunk` and if `stack_length == 0` to match the
upstream parser. Otherwise, this optimization is skipped.
* Subtracts the index, otherwise it will skip the hash allocation for
the following: `keyword(*empty_array, a: 2, **empty_hash)`.
* Sets `dup_rest` in order to determine when to set the correct flags
* Doesn't set `VM_CALL_KW_SPLAT_MUT` flag unless `dup_rest` doesn't
match `initial_dup_rest`.
Given the following code:
```ruby
keyword(*empty_array, a: 2)
```
Instructions before:
```
== disasm: #<ISeq:[email protected]:4 (4,0)-(8,3)>
local table (size: 2, argc: 1 [opts: 0, rest: -1, post: 0, block: -1, kw: -1@-1, kwrest: -1])
[ 2] empty_hash@0<Arg>[ 1] empty_array@1
0000 newarray 0 ( 5)[LiCa]
0002 setlocal_WC_0 empty_array@1
0004 putself ( 7)[Li]
0005 getlocal_WC_0 empty_array@1
0007 splatarray true
0009 putobject :a
0011 putobject 2
0013 newhash 2
0015 opt_send_without_block <calldata!mid:keyword, argc:2, ARGS_SPLAT|ARGS_SPLAT_MUT|FCALL|KW_SPLAT>
0017 leave ( 8)[Re]
```
Instructions after:
```
== disasm: #<ISeq:[email protected]:4 (4,0)-(8,3)>
local table (size: 2, argc: 1 [opts: 0, rest: -1, post: 0, block: -1, kw: -1@-1, kwrest: -1])
[ 2] empty_hash@0<Arg>[ 1] empty_array@1
0000 newarray 0 ( 5)[LiCa]
0002 setlocal_WC_0 empty_array@1
0004 putself ( 7)[Li]
0005 getlocal_WC_0 empty_array@1
0007 splatarray false
0009 putobject {:a=>2}
0011 opt_send_without_block <calldata!mid:keyword, argc:2, ARGS_SPLAT|FCALL|KW_SPLAT>
0013 leave ( 8)[Re]
```
Differences:
* `splatarray` is `false` not `true
* `putobject`, `putobject`, `newhash` is simply `putobject` with
optimizations on
* Remove `ARGS_SPLAT_MUT` flag
Related: ruby/prism#2994
Co-authored-by: Kevin Newton <[email protected]>
Notes:
Merged: https://github.com/ruby/ruby/pull/11438
|
|
Notes:
Merged: https://github.com/ruby/ruby/pull/11425
|
|
If an array element is a static literal that does not result in a
intermediate array, it still needs to be compiled normally.
Notes:
Merged: https://github.com/ruby/ruby/pull/11426
|
|
Related: ruby/prism#2935
Notes:
Merged: https://github.com/ruby/ruby/pull/11415
|
|
It just wastes memory.
Notes:
Merged: https://github.com/ruby/ruby/pull/11361
|
|
Notes:
Merged: https://github.com/ruby/ruby/pull/11236
|
|
In the following code:
```ruby
def foo(&blk)
blk.call
end
```
Prism was using the `getblockparam` instruction but it should be
`getblockparamproxy`. In this case we have a receiver, if it's a local
variable read node, then it needs to go through the path to use
`getblockparamproxy`.
Before:
```
== disasm: #<ISeq:<main>@test2.rb:1 (1,0)-(3,3)>
0000 definemethod :foo, foo ( 1)[Li]
0003 putobject :foo
0005 leave
== disasm: #<ISeq:[email protected]:1 (1,0)-(3,3)>
local table (size: 1, argc: 0 [opts: 0, rest: -1, post: 0, block: 0, kw: -1@-1, kwrest: -1])
[ 1] blk@0<Block>
0000 getblockparam blk@0, 0 ( 2)[LiCa]
0003 opt_send_without_block <calldata!mid:call, argc:0, ARGS_SIMPLE>
0005 leave ( 3)[Re]
```
After:
```
== disasm: #<ISeq:<main>@test2.rb:1 (1,0)-(3,3)>
0000 definemethod :foo, foo ( 1)[Li]
0003 putobject :foo
0005 leave
== disasm: #<ISeq:[email protected]:1 (1,0)-(3,3)>
local table (size: 1, argc: 0 [opts: 0, rest: -1, post: 0, block: 0, kw: -1@-1, kwrest: -1])
[ 1] blk@0<Block>
0000 getblockparamproxy blk@0, 0 ( 2)[LiCa]
0003 opt_send_without_block <calldata!mid:call, argc:0, ARGS_SIMPLE>
0005 leave ( 3)[Re]
```
Fixes `test_getblockparamproxy` and `test_ifunc_getblockparamproxy` in
test_yjit.rb. Related to ruby/prism#2935.
Notes:
Merged: https://github.com/ruby/ruby/pull/11231
|
|
This addresses one of the issues in the `test_kw_splat_nil` failure, but
doesn't make the test pass because of other changes that need to be made
to Prism directly.
One issue was when we have the following code Prism was using
`putobject` with an empty hash whereas the parse.y parser used `putnil`.
```ruby
:ok.itself(**nil)
```
Before:
```
0000 putobject :ok ( 1)[Li]
0002 putobject {}
0004 opt_send_without_block <calldata!mid:itself, argc:1, KW_SPLAT>
0006 leave
```
After:
```
== disasm: #<ISeq:<main>@test2.rb:1 (1,0)-(1,17)>
0000 putobject :ok ( 1)[Li]
0002 putnil
0003 opt_send_without_block <calldata!mid:itself, argc:1, KW_SPLAT>
0005 leave
```
Related to ruby/prism#2935.
Notes:
Merged: https://github.com/ruby/ruby/pull/11232
|
|
Notes:
Merged: https://github.com/ruby/ruby/pull/11228
|
|
Notes:
Merged: https://github.com/ruby/ruby/pull/11222
|
|
For example, the following code leaks:
code = 1000.times.map { |i| "var#{i} = 1" }.join("\n")
10.times do
1000.times do
RubyVM::InstructionSequence.compile_prism(code)
end
puts `ps -o rss= -p #{$$}`
end
Before:
70384
88032
103856
115712
125584
132768
144784
152624
165296
180608
After:
62368
78784
74512
87712
85072
77728
69424
74992
71264
81440
Notes:
Merged: https://github.com/ruby/ruby/pull/11222
|
|
prism_compile.c:5770:40: warning: comparison of integer expressions of different signedness: ‘size_t’ {aka ‘long unsigned int’} and ‘int’ [-Wsign-compare]
5770 | if (tmp_array_size >= min_tmp_array_size) {
| ^~
|
|
prism_compile.c:1406:27: warning: comparison of integer expressions of different signedness: ‘size_t’ {aka ‘long unsigned int’} and ‘int’ [-Wsign-compare]
1406 | if (count >= min_tmp_hash_length) {
| ^~
|
|
If the symbol node is interpolated like this `:"#{foo}"` the instruction
sequence should be `putstring` followed by `intern`. In this case it was
a `putobject` causing the `test_yjit` tests to fail. Note that yjit is
not required to reproduce - the instructions are `putstring` and
`intern` for yjit and non-yjit with the original parser.
To fix I moved `pm_interpolated_node_compile` out of the else, and
entirely removed the conditional. `pm_interpolated_node_compile` knows
how / when to use `putstring` over `putobject` already. The `intern` is
then added by removing the conditional.
Before:
```
== disasm: #<ISeq:<main>@test2.rb:1 (1,0)-(1,11)>
0000 putobject :foo ( 1)[Li]
0002 leave
```
After:
```
== disasm: #<ISeq:<main>@test2.rb:1 (1,0)-(1,11)>
0000 putstring "foo" ( 1)[Li]
0002 intern
0003 leave
```
Fixes the test `TestYJIT#test_compile_dynamic_symbol`. Related to ruby/prism#2935
Notes:
Merged: https://github.com/ruby/ruby/pull/11191
|
|
Notes:
Merged: https://github.com/ruby/ruby/pull/11200
|
|
Notes:
Merged: https://github.com/ruby/ruby/pull/11200
|
|
When we have an empty hash the iseq should have a `newhash` but instead
had a `duphash`. To fix, check if the node's elements are equal to `0`.
If so we want a `newhash`, otherwise use the original `duphash`
instructions.
Before:
```
== disasm: #<ISeq:<main>@test2.rb:1 (1,0)-(1,2)>
0000 duphash {} ( 1)[Li]
0002 leave
```
After:
```
== disasm: #<ISeq:<main>@test2.rb:1 (1,0)-(1,2)>
0000 newhash 0 ( 1)[Li]
0002 leave
```
Fixes the test `TestYJIT#test_compile_newhash`. Related to ruby/prism#2935
Notes:
Merged: https://github.com/ruby/ruby/pull/11190
|
|
|
|
|
|
Notes:
Merged: https://github.com/ruby/ruby/pull/11163
|
|
Notes:
Merged: https://github.com/ruby/ruby/pull/11188
|
|
Notes:
Merged: https://github.com/ruby/ruby/pull/11177
|
|
Notes:
Merged: https://github.com/ruby/ruby/pull/11177
|
|
|
|
|
|
|
|
Co-authored-by: Adam Hess <[email protected]>
|
|
|
|
|
|
|
|
|
|
While working on a separate issue we found that in some cases
`ary_heap_realloc` was being called on frozen arrays. To fix this, this
change does the following:
1) Updates `rb_ary_freeze` to assert the type is an array, return if
already frozen, and shrink the capacity if it is not embedded, shared
or a shared root.
2) Replaces `rb_obj_freeze` with `rb_ary_freeze` when the object is
always an array.
3) In `ary_heap_realloc`, ensure the new capa is set with
`ARY_SET_CAPA`. Previously the change in capa was not set.
4) Adds an assertion to `ary_heap_realloc` that the array is not frozen.
Some of this work was originally done in
https://github.com/ruby/ruby/pull/2640, referencing this issue
https://bugs.ruby-lang.org/issues/16291. There didn't appear to be any
objections to this PR, it appears to have simply lost traction.
The original PR made changes to arrays and strings at the same time,
this PR only does arrays. Also it was old enough that rather than revive
that branch I've made a new one. I added Lourens as co-author in addtion
to Aaron who helped me with this patch.
The original PR made this change for performance reasons, and while
that's still true for this PR, the goal of this PR is to avoid
calling `ary_heap_realloc` on frozen arrays. The capacity should be
shrunk _before_ the array is frozen, not after.
Co-authored-by: Aaron Patterson <[email protected]>
Co-Authored-By: methodmissing <[email protected]>
|
|
|
|
This supports the nodes in both in the parse.y and prism compilers.
Fixes [Bug #20043]
Co-authored-by: Kevin Newton <[email protected]>
|
|
This commit adds `sendforward` and `invokesuperforward` for forwarding
parameters to calls
Co-authored-by: Matt Valentine-House <[email protected]>
|
|
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 <[email protected]>
Co-Authored-By: Alan Wu <[email protected]>
|
|
|
|
|
|
|
|
|
