[ruby-core:100631] [Ruby master Feature#17291] Optimize __send__ call

[ruby-core:100631] [Ruby master Feature#17291] Optimize __send__ call

[muraken]

Issue #17291 has been reported by mrkn (Kenta Murata).

----------------------------------------
Feature #17291: Optimize __send__ call
https://bugs.ruby-lang.org/issues/17291

* Author: mrkn (Kenta Murata)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
----------------------------------------
I made a patch to optimize a `__send__` call. This optimization replaces a `__send__` method call with a call of the method whose name is the first argument of `__send__` method. The patch is available in [this pull-request](https://github.com/ruby/ruby/pull/3720).

By this change, the redefined `__send__` method is no longer called when it is called by a symbol method name. I guess it is no problem because the following warning message is displayed for a long time.

    $ ruby -e 'def __send__; end'
    -e:1: warning: redefining `__send__' may cause serious problems

This proposal introduces two new instructions: `sendsym` and `opt_sendsym_without_block`.  These instructions handle the cases that the first argument of `__send__` method is not a symbol literal.  I think I can combine these two instructions into one if prefered.

This proposal includes the change proposed in #17288.  I'll mark it as a duplicate of this proposal.

I don't handle `send` method in this proposal. The reason is that we need to examine the redefinition of `send` method in the instruction execution time. I want to discuss only `__send__` method in this ticket.

The benchmark result is below:

```
# Iteration per second (i/s)

|                 |compare-ruby|built-ruby|
|:----------------|-----------:|---------:|
|vm_send_sym      |     18.001M|  112.208M|
|                 |           -|     6.23x|
|vm_send_var      |     17.779M|   30.922M|
|                 |           -|     1.74x|
|vm_send_var_alt  |      3.817M|    6.817M|
|                 |           -|     1.79x|
```



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

[ruby-core:100664] [Ruby master Feature#17291] Optimize __send__ call

[shyouhei]

Issue #17291 has been updated by shyouhei (Shyouhei Urabe).


I'm neutral (at least no against it).  `__send__` in general has other usages than to reroute method visibilities. Optimising it could benefit good wills.

----------------------------------------
Feature #17291: Optimize __send__ call
https://bugs.ruby-lang.org/issues/17291#change-88304

* Author: mrkn (Kenta Murata)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
----------------------------------------
I made a patch to optimize a `__send__` call. This optimization replaces a `__send__` method call with a call of the method whose name is the first argument of `__send__` method. The patch is available in [this pull-request](https://github.com/ruby/ruby/pull/3720).

By this change, the redefined `__send__` method is no longer called when it is called by a symbol method name. I guess it is no problem because the following warning message is displayed for a long time.

    $ ruby -e 'def __send__; end'
    -e:1: warning: redefining `__send__' may cause serious problems

This proposal introduces two new instructions: `sendsym` and `opt_sendsym_without_block`.  These instructions handle the cases that the first argument of `__send__` method is not a symbol literal.  I think I can combine these two instructions into one if prefered.

This proposal includes the change proposed in #17288.  I'll mark it as a duplicate of this proposal.

I don't handle `send` method in this proposal. The reason is that we need to examine the redefinition of `send` method in the instruction execution time. I want to discuss only `__send__` method in this ticket.

The benchmark result is below:

```
# Iteration per second (i/s)

|                 |compare-ruby|built-ruby|
|:----------------|-----------:|---------:|
|vm_send_sym      |     18.001M|  112.208M|
|                 |           -|     6.23x|
|vm_send_var      |     17.779M|   30.922M|
|                 |           -|     1.74x|
|vm_send_var_alt  |      3.817M|    6.817M|
|                 |           -|     1.79x|
```



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

[ruby-core:100707] [Ruby master Feature#17291] Optimize __send__ call

[muraken]

Issue #17291 has been updated by mrkn (Kenta Murata).


I found that rspec-core redefines `__send__`.

https://github.com/rspec/rspec-mocks/blob/461d7f6f7f688f69154e410dcd9c7690120e7dbf/lib/rspec/mocks/verifying_double.rb#L45-L53

----------------------------------------
Feature #17291: Optimize __send__ call
https://bugs.ruby-lang.org/issues/17291#change-88351

* Author: mrkn (Kenta Murata)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
----------------------------------------
I made a patch to optimize a `__send__` call. This optimization replaces a `__send__` method call with a call of the method whose name is the first argument of `__send__` method. The patch is available in [this pull-request](https://github.com/ruby/ruby/pull/3720).

By this change, the redefined `__send__` method is no longer called when it is called by a symbol method name. I guess it is no problem because the following warning message is displayed for a long time.

    $ ruby -e 'def __send__; end'
    -e:1: warning: redefining `__send__' may cause serious problems

This proposal introduces two new instructions: `sendsym` and `opt_sendsym_without_block`.  These instructions handle the cases that the first argument of `__send__` method is not a symbol literal.  I think I can combine these two instructions into one if prefered.

This proposal includes the change proposed in #17288.  I'll mark it as a duplicate of this proposal.

I don't handle `send` method in this proposal. The reason is that we need to examine the redefinition of `send` method in the instruction execution time. I want to discuss only `__send__` method in this ticket.

The benchmark result is below:

```
# Iteration per second (i/s)

|                 |compare-ruby|built-ruby|
|:----------------|-----------:|---------:|
|vm_send_sym      |     18.001M|  112.208M|
|                 |           -|     6.23x|
|vm_send_var      |     17.779M|   30.922M|
|                 |           -|     1.74x|
|vm_send_var_alt  |      3.817M|    6.817M|
|                 |           -|     1.79x|
```



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

[ruby-core:100709] [Ruby master Feature#17291] Optimize __send__ call

[shyouhei]

Issue #17291 has been updated by shyouhei (Shyouhei Urabe).


It seems this leaks memory?

```ruby
`nproc --all`.to_i.times.map do |i|
  Ractor.new i do |j|
    sleep rand
    k = 0
    while true do
      __send__ :"#{j}_#{k+=1}" rescue nil
      printf("%d\n", GC.stat(:heap_live_slots)) if (k % 65535) == 0
    end
  end
end.map(&:take)
```

I see very different output comparing the proposed implementation versus master.

----------------------------------------
Feature #17291: Optimize __send__ call
https://bugs.ruby-lang.org/issues/17291#change-88353

* Author: mrkn (Kenta Murata)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
----------------------------------------
I made a patch to optimize a `__send__` call. This optimization replaces a `__send__` method call with a call of the method whose name is the first argument of `__send__` method. The patch is available in [this pull-request](https://github.com/ruby/ruby/pull/3720).

By this change, the redefined `__send__` method is no longer called when it is called by a symbol method name. I guess it is no problem because the following warning message is displayed for a long time.

    $ ruby -e 'def __send__; end'
    -e:1: warning: redefining `__send__' may cause serious problems

This proposal introduces two new instructions: `sendsym` and `opt_sendsym_without_block`.  These instructions handle the cases that the first argument of `__send__` method is not a symbol literal.  I think I can combine these two instructions into one if prefered.

This proposal includes the change proposed in #17288.  I'll mark it as a duplicate of this proposal.

I don't handle `send` method in this proposal. The reason is that we need to examine the redefinition of `send` method in the instruction execution time. I want to discuss only `__send__` method in this ticket.

The benchmark result is below:

```
# Iteration per second (i/s)

|                 |compare-ruby|built-ruby|
|:----------------|-----------:|---------:|
|vm_send_sym      |     18.001M|  112.208M|
|                 |           -|     6.23x|
|vm_send_var      |     17.779M|   30.922M|
|                 |           -|     1.74x|
|vm_send_var_alt  |      3.817M|    6.817M|
|                 |           -|     1.79x|
```



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

[ruby-core:100712] [Ruby master Feature#17291] Optimize __send__ call

[eregontp]

Issue #17291 has been updated by Eregon (Benoit Daloze).


Yeah I don't think a warning is good enough to prevent people overriding it.
It should be an exception if the goal is to prevent overriding it.

I think we should not compromise semantics for optimizations, that usually leads to more complicated semantics that alternative Ruby implementations have to replicate (which is nonsense if those implementations would check it correctly if redefined).
As an example, the optimization for Hash#each_pair led to very confusing semantics where lambdas/Method#to_proc appear to unsplat arguments (they do not, it's just a side effect of the incorrect optimization).

----------------------------------------
Feature #17291: Optimize __send__ call
https://bugs.ruby-lang.org/issues/17291#change-88356

* Author: mrkn (Kenta Murata)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
----------------------------------------
I made a patch to optimize a `__send__` call. This optimization replaces a `__send__` method call with a call of the method whose name is the first argument of `__send__` method. The patch is available in [this pull-request](https://github.com/ruby/ruby/pull/3720).

By this change, the redefined `__send__` method is no longer called when it is called by a symbol method name. I guess it is no problem because the following warning message is displayed for a long time.

    $ ruby -e 'def __send__; end'
    -e:1: warning: redefining `__send__' may cause serious problems

This proposal introduces two new instructions: `sendsym` and `opt_sendsym_without_block`.  These instructions handle the cases that the first argument of `__send__` method is not a symbol literal.  I think I can combine these two instructions into one if prefered.

This proposal includes the change proposed in #17288.  I'll mark it as a duplicate of this proposal.

I don't handle `send` method in this proposal. The reason is that we need to examine the redefinition of `send` method in the instruction execution time. I want to discuss only `__send__` method in this ticket.

The benchmark result is below:

```
# Iteration per second (i/s)

|                 |compare-ruby|built-ruby|
|:----------------|-----------:|---------:|
|vm_send_sym      |     18.001M|  112.208M|
|                 |           -|     6.23x|
|vm_send_var      |     17.779M|   30.922M|
|                 |           -|     1.74x|
|vm_send_var_alt  |      3.817M|    6.817M|
|                 |           -|     1.79x|
```



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

[ruby-core:100728] [Ruby master Feature#17291] Optimize __send__ call

[muraken]

Issue #17291 has been updated by mrkn (Kenta Murata).


shyouhei (Shyouhei Urabe) wrote in #note-4:
> It seems this leaks memory?
> 
> ```ruby
> `nproc --all`.to_i.times.map do |i|
>   Ractor.new :"#{i}_0" do |sym|
>     while true do
>       __send__(sym = sym.succ) rescue nil 
>     end
>   end
> end
> 
> while true do
>   sleep 1
>   GC.start
>   p GC.stat(:heap_live_slots)
> end
> ```
> 
> I see very different output comparing the proposed implementation versus master.

I used `SYM2ID` in `compile_call` function and `sendsym` and `opt_sendsym_without_block` instructions.  This `SYM2ID` makes dynamic symbols permanent, so many symbols remained in the heap.  This is the reason for the observed phenomenon.

I added a commit to fix this bug.

----------------------------------------
Feature #17291: Optimize __send__ call
https://bugs.ruby-lang.org/issues/17291#change-88371

* Author: mrkn (Kenta Murata)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
----------------------------------------
I made a patch to optimize a `__send__` call. This optimization replaces a `__send__` method call with a call of the method whose name is the first argument of `__send__` method. The patch is available in [this pull-request](https://github.com/ruby/ruby/pull/3720).

By this change, the redefined `__send__` method is no longer called when it is called by a symbol method name. I guess it is no problem because the following warning message is displayed for a long time.

    $ ruby -e 'def __send__; end'
    -e:1: warning: redefining `__send__' may cause serious problems

This proposal introduces two new instructions: `sendsym` and `opt_sendsym_without_block`.  These instructions handle the cases that the first argument of `__send__` method is not a symbol literal.  I think I can combine these two instructions into one if prefered.

This proposal includes the change proposed in #17288.  I'll mark it as a duplicate of this proposal.

I don't handle `send` method in this proposal. The reason is that we need to examine the redefinition of `send` method in the instruction execution time. I want to discuss only `__send__` method in this ticket.

The benchmark result is below:

```
# Iteration per second (i/s)

|                 |compare-ruby|built-ruby|
|:----------------|-----------:|---------:|
|vm_send_sym      |     18.001M|  112.208M|
|                 |           -|     6.23x|
|vm_send_var      |     17.779M|   30.922M|
|                 |           -|     1.74x|
|vm_send_var_alt  |      3.817M|    6.817M|
|                 |           -|     1.79x|
```



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

[ruby-core:100729] [Ruby master Feature#17291] Optimize __send__ call

[muraken]

Issue #17291 has been updated by mrkn (Kenta Murata).


The new benchmark result is below:

```
# Iteration per second (i/s)

|                     |compare-ruby|built-ruby|
|:--------------------|-----------:|---------:|
|vm_send_sym          |     18.265M|  113.593M|
|                     |           -|     6.22x|
|vm_send_var          |     17.750M|   31.974M|
|                     |           -|     1.80x|
|vm_send_var_alt      |      3.955M|    7.499M|
|                     |           -|     1.90x|
|vm_send_sym_missing  |      7.135M|    8.982M|
|                     |           -|     1.26x|
|vm_send_var_missing  |      7.271M|    7.454M|
|                     |           -|     1.03x|
```

----------------------------------------
Feature #17291: Optimize __send__ call
https://bugs.ruby-lang.org/issues/17291#change-88373

* Author: mrkn (Kenta Murata)
* Status: Assigned
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
----------------------------------------
I made a patch to optimize a `__send__` call. This optimization replaces a `__send__` method call with a call of the method whose name is the first argument of `__send__` method. The patch is available in [this pull-request](https://github.com/ruby/ruby/pull/3720).

By this change, the redefined `__send__` method is no longer called when it is called by a symbol method name. I guess it is no problem because the following warning message is displayed for a long time.

    $ ruby -e 'def __send__; end'
    -e:1: warning: redefining `__send__' may cause serious problems

This proposal introduces two new instructions: `sendsym` and `opt_sendsym_without_block`.  These instructions handle the cases that the first argument of `__send__` method is not a symbol literal.  I think I can combine these two instructions into one if prefered.

This proposal includes the change proposed in #17288.  I'll mark it as a duplicate of this proposal.

I don't handle `send` method in this proposal. The reason is that we need to examine the redefinition of `send` method in the instruction execution time. I want to discuss only `__send__` method in this ticket.

The benchmark result is below:

```
# Iteration per second (i/s)

|                 |compare-ruby|built-ruby|
|:----------------|-----------:|---------:|
|vm_send_sym      |     18.001M|  112.208M|
|                 |           -|     6.23x|
|vm_send_var      |     17.779M|   30.922M|
|                 |           -|     1.74x|
|vm_send_var_alt  |      3.817M|    6.817M|
|                 |           -|     1.79x|
```



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

[ruby-core:102019] [Ruby master Feature#17291] Optimize __send__ call

[muraken]

Issue #17291 has been updated by mrkn (Kenta Murata).


mrkn (Kenta Murata) wrote in #note-3:
> I found that rspec-core redefines `__send__`.
> 
> https://github.com/rspec/rspec-mocks/blob/461d7f6f7f688f69154e410dcd9c7690120e7dbf/lib/rspec/mocks/verifying_double.rb#L45-L53

This redefinition is used to distinguish the form of the method call in a mock object.
rspec-mocks recognizes that the form of `recv.__send__(:meth)` is used when `__send__` is called before `method_missing` is called, or the form of `recv.meth` is used when otherwise.

The feature to distinguish method calling form is necessary to keep the compatibility if we employ this optimization of `__send__` call.

----------------------------------------
Feature #17291: Optimize __send__ call
https://bugs.ruby-lang.org/issues/17291#change-89867

* Author: mrkn (Kenta Murata)
* Status: Assigned
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
----------------------------------------
I made a patch to optimize a `__send__` call. This optimization replaces a `__send__` method call with a call of the method whose name is the first argument of `__send__` method. The patch is available in [this pull-request](https://github.com/ruby/ruby/pull/3720).

By this change, the redefined `__send__` method is no longer called when it is called by a symbol method name. I guess it is no problem because the following warning message is displayed for a long time.

    $ ruby -e 'def __send__; end'
    -e:1: warning: redefining `__send__' may cause serious problems

This proposal introduces two new instructions: `sendsym` and `opt_sendsym_without_block`.  These instructions handle the cases that the first argument of `__send__` method is not a symbol literal.  I think I can combine these two instructions into one if prefered.

This proposal includes the change proposed in #17288.  I'll mark it as a duplicate of this proposal.

I don't handle `send` method in this proposal. The reason is that we need to examine the redefinition of `send` method in the instruction execution time. I want to discuss only `__send__` method in this ticket.

The benchmark result is below:

```
# Iteration per second (i/s)

|                 |compare-ruby|built-ruby|
|:----------------|-----------:|---------:|
|vm_send_sym      |     18.001M|  112.208M|
|                 |           -|     6.23x|
|vm_send_var      |     17.779M|   30.922M|
|                 |           -|     1.74x|
|vm_send_var_alt  |      3.817M|    6.817M|
|                 |           -|     1.79x|
```



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