Project

General

Profile

Feature #17291

Optimize __send__ call

Added by mrkn (Kenta Murata) about 1 month ago. Updated 24 days ago.

Status:
Assigned
Priority:
Normal
Target version:
-
[ruby-core:100631]

Description

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.

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|

Related issues

Has duplicate Ruby master - Feature #17288: Optimize __send__ call with a literal method nameOpenmatz (Yukihiro Matsumoto)Actions
#1

Updated by mrkn (Kenta Murata) about 1 month ago

  • Has duplicate Feature #17288: Optimize __send__ call with a literal method name added

Updated by shyouhei (Shyouhei Urabe) about 1 month ago

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.

Updated by shyouhei (Shyouhei Urabe) 26 days ago

It seems this leaks memory?

`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.

Updated by Eregon (Benoit Daloze) 26 days ago

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/destructure arguments (they do not, it's just a side effect of the incorrect optimization).

Updated by mrkn (Kenta Murata) 24 days ago

shyouhei (Shyouhei Urabe) wrote in #note-4:

It seems this leaks memory?

`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.

#7

Updated by mrkn (Kenta Murata) 24 days ago

  • Status changed from Open to Assigned

Updated by mrkn (Kenta Murata) 24 days ago

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|

Also available in: Atom PDF