Project

General

Profile

Feature #13715

[PATCH] avoid garbage from Symbol#to_s in interpolation

Added by normalperson (Eric Wong) about 1 year ago. Updated about 1 year ago.

Status:
Open
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:81905]

Description

"ruby -e 'p GC.stat(:total_allocated_objects)'" goes from
70199 to 69540 allocated objects when loading RubyGems from
a clean install.

The increased VM size slows down the whileloop2 and vm2_dstr
case slightly, but string interpolation often consists of
non-strings.  The addition of inline cache helps integer cases
slightly, and the intended Symbol optimization gives a major
improvement.

speedup relative to trunk
name           |built
---------------|------:
loop_whileloop2|  0.984
vm2_dstr*      |  0.991
vm2_dstr_digit*|  1.167
vm2_dstr_int*  |  1.120
vm2_dstr_nil*  |  1.181
vm2_dstr_sym*  |  1.663

Digits (0-9), Integers, and perhaps true/false/nil may be
optimized in the future.

* vm_eval.c (rb_vm_call0_body): new function exports vm_call0_body
* vm_insnshelper.c (vm_tostring): new function
* insns.def (tostring): call vm_tostring with ci + cc
* compile.c (iseq_compile_each0): adjust tostring insn compile
* benchmark/bm_vm2_dstr_digit.rb: new benchmark
* benchmark/bm_vm2_dstr_int.rb: ditto
* benchmark/bm_vm2_dstr_nil.rb: ditto
* benchmark/bm_vm2_dstr_sym.rb: ditto

History

#1 [ruby-core:82017] Updated by ko1 (Koichi Sasada) about 1 year ago

VALUE rb_vm_call0_body(rb_thread_t *th, struct rb_calling_info *calling,

You don't need to expose vm_call0_body() because vm_eval.c and vm_insnhelper.c are included in vm.c.

    if (RB_TYPE_P(recv, T_SYMBOL)) {
    vm_search_method(ci, cc, recv);

It seems we can use vm_method_cfunc_is().

    calling.block_handler = VM_BLOCK_HANDLER_NONE;
    calling.argc = 0;
    calling.recv = recv;
    val = rb_vm_call0_body(th, &calling, ci, cc, 0);
    return RB_TYPE_P(val, T_STRING) ? val : rb_any_to_s(recv);

How about to call rb_obj_as_string() directly? I understand that you want to reuse method search results, but code will be simplified.

Or make new function to call method with given ci, cc?

#2 [ruby-core:82027] Updated by normalperson (Eric Wong) about 1 year ago

ko1@atdot.net wrote:

VALUE rb_vm_call0_body(rb_thread_t *th, struct rb_calling_info *calling,

You don't need to expose vm_call0_body() because vm_eval.c and vm_insnhelper.c are included in vm.c.

Ah, thanks. I just added a prototype for vm_call0_body.

    if (RB_TYPE_P(recv, T_SYMBOL)) {
 vm_search_method(ci, cc, recv);

It seems we can use vm_method_cfunc_is().

Right, I expanded the function in my original patch it since I
wanted to make it obvious that cc is populated regardless of
function match. I am using a comment instead, now.

    calling.block_handler = VM_BLOCK_HANDLER_NONE;
    calling.argc = 0;
    calling.recv = recv;
    val = rb_vm_call0_body(th, &calling, ci, cc, 0);
    return RB_TYPE_P(val, T_STRING) ? val : rb_any_to_s(recv);

How about to call rb_obj_as_string() directly? I understand that you want to reuse method search results, but code will be simplified.

I think the >10% improvement for non-symbol dstr benchmarks
is worth the complexity, especially since we populate cc for
vm_method_cfunc_is anyways.

Or make new function to call method with given ci, cc?

I'm not sure what you mean, vm_call0_body is insufficient?

Anyways, v2 patch here:

https://80x24.org/spew/20170713075445.25252-1-e@80x24.org/raw

Also available in: Atom PDF