Fast path for iseq-to-iseq bmethod call #6428
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, when a Ruby caller calls a Ruby block method (defined by `define_method`, etc.), we used the code for calling Ruby block method from C code. This involved argument copying and re-entering `vm_exec_core` which was extra work. This used to be necessary for firing TracePoint events, but that issue was resolved in 9121e57. This patch reuse the interpreter invocation we're already in and avoids the extra argument copying for this situation. benchmark/vm_defined_method.yml shows a nice improvement. patched: 4952460.5 i/s master: 2490187.6 i/s - 1.99x slower
nobu
changed the title
XrXr
added a commit
to XrXr/ruby
that referenced
this pull request
Jul 12, 2023
Previously, when a Ruby caller calls a Ruby block method (defined by `define_method`, etc.), we used the code for calling Ruby block method from C code. This involved argument shifting and re-entering vm_exec_core() which was more work than necessary. Re-entering vm_exec_core() used to be necessary for firing TracePoint events, but that's no longer the case since 9121e57. This patch reuse the interpreter invocation we're already in and avoids the argument shifting for this situation. It uses the same stack layout as the JITs and so is using VM_FRAME_FLAG_STACK_RECV. C-to-ISeq bmethod calls continue to use a different layout (See invoke_iseq_block_from_c()). benchmark/vm_defined_method.yml shows a nice improvement. patched: 7578743.1 i/s master: 4796596.3 i/s - 1.58x slower Closes ruby#6428
XrXr
added a commit
to XrXr/ruby
that referenced
this pull request
Jul 12, 2023
Previously, when a Ruby caller calls a Ruby block method (defined by `define_method`, etc.), we used the code for calling Ruby block method from C code. This involved argument shifting and re-entering vm_exec_core() which was more work than necessary. Re-entering vm_exec_core() used to be necessary for firing TracePoint events, but that's no longer the case since 9121e57. This patch reuse the interpreter invocation we're already in and avoids the argument shifting for this situation. It uses the same stack layout as the JITs and so is using VM_FRAME_FLAG_STACK_RECV. C-to-ISeq bmethod calls continue to use a different layout (See invoke_iseq_block_from_c()). benchmark/vm_defined_method.yml shows a nice improvement. patched: 7578743.1 i/s master: 4796596.3 i/s - 1.58x slower Closes ruby#6428
XrXr
added a commit
to XrXr/ruby
that referenced
this pull request
Jul 17, 2023
This commit removes the __bp__ field from rb_control_frame_t. It was
introduced to help MJIT, but since MJIT was replaced by RJIT, we can use
vm_base_ptr() to compute it from the SP of the previous control frame
instead. Removing the field avoids needing to set it up when pushing new
frames.
Simply removing __bp__ would cause crashes since RJIT and YJIT used a
slightly different stack layout for bmethod calls than the interpreter.
At the moment of the call, the two layouts looked as follows:
┌────────────┐ ┌────────────┐
│ frame_base │ │ frame_base │
├────────────┤ ├────────────┤
│ ... │ │ ... │
├────────────┤ ├────────────┤
│ args │ │ args │
├────────────┤ └────────────┘<─prev_frame_sp
│ receiver │
prev_frame_sp─>└────────────┘
RJIT & YJIT interpreter
Essentially, vm_base_ptr() needs to compute the address to frame_base
given prev_frame_sp in the diagrams. The presence of the receiver
created an off-by-one situation.
Make the interpreter use the layout the JITs use for iseq-to-iseq
bmethod calls. Doing so removes unnecessary argument shifting and
vm_exec_core() re-entry from the interpreter, yielding a speed
improvement visible through `benchmark/vm_defined_method.yml`:
patched: 7578743.1 i/s
master: 4796596.3 i/s - 1.58x slower
C-to-iseq bmethod calls now store one more VALUE than before, but that
should have negligible impact on overall performance.
Note that re-entering vm_exec_core() used to be necessary for firing
TracePoint events, but that's no longer the case since
9121e57.
Closes ruby#6428
maximecb
pushed a commit
that referenced
this pull request
Jul 17, 2023
Remove rb_control_frame_t::__bp__ and optimize bmethod calls
This commit removes the __bp__ field from rb_control_frame_t. It was
introduced to help MJIT, but since MJIT was replaced by RJIT, we can use
vm_base_ptr() to compute it from the SP of the previous control frame
instead. Removing the field avoids needing to set it up when pushing new
frames.
Simply removing __bp__ would cause crashes since RJIT and YJIT used a
slightly different stack layout for bmethod calls than the interpreter.
At the moment of the call, the two layouts looked as follows:
┌────────────┐ ┌────────────┐
│ frame_base │ │ frame_base │
├────────────┤ ├────────────┤
│ ... │ │ ... │
├────────────┤ ├────────────┤
│ args │ │ args │
├────────────┤ └────────────┘<─prev_frame_sp
│ receiver │
prev_frame_sp─>└────────────┘
RJIT & YJIT interpreter
Essentially, vm_base_ptr() needs to compute the address to frame_base
given prev_frame_sp in the diagrams. The presence of the receiver
created an off-by-one situation.
Make the interpreter use the layout the JITs use for iseq-to-iseq
bmethod calls. Doing so removes unnecessary argument shifting and
vm_exec_core() re-entry from the interpreter, yielding a speed
improvement visible through `benchmark/vm_defined_method.yml`:
patched: 7578743.1 i/s
master: 4796596.3 i/s - 1.58x slower
C-to-iseq bmethod calls now store one more VALUE than before, but that
should have negligible impact on overall performance.
Note that re-entering vm_exec_core() used to be necessary for firing
TracePoint events, but that's no longer the case since
9121e57.
Closes #6428
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, when a Ruby caller calls a Ruby block method (defined by
define_method, etc.), we used the code for calling Ruby block method from C code. This involved argument copying and re-enteringvm_exec_corewhich was extra work. This used to be necessary for firing TracePoint events, but that issue was resolved in 9121e57.This patch reuse the interpreter invocation we're already in and avoids the extra argument copying for this situation.
benchmark/vm_defined_method.yml shows a nice improvement.