[ruby-core:105743] [Ruby master Feature#13715] [PATCH] avoid garbage from Symbol#to_s in interpolation

[ruby-core:105743] [Ruby master Feature#13715] [PATCH] avoid garbage from Symbol#to_s in interpolation

[jeremyevans0 (Jeremy Evans)]

Issue #13715 has been updated by jeremyevans0 (Jeremy Evans).


I've submitted a pull request for an updated version of @normalperson's patch: https://github.com/ruby/ruby/pull/5002, expanding the optimization to include nil/true/false/0-9.  This is an across-the-board performance improvement, mostly by reducing the number of VM instructions from 6 to 1. From the benchmarks included in the patch, performance increase from the patch is roughly:

:symbol :: 60%
0-9 :: 50%
nil/true/false :: 20%
integer :: 10%
[] :: 10%
"" :: 3%

It's definitely adds complexity, and I'm not sure how much of a difference it makes in a production application.  However, string interpolation is fairly common in most Ruby applications and libraries, and many interpolations will use symbols/nil/true/false/0-9, so I think it is worthy of consideration.

----------------------------------------
Feature #13715: [PATCH] avoid garbage from Symbol#to_s in interpolation
https://bugs.ruby-lang.org/issues/13715#change-94246

* Author: normalperson (Eric Wong)
* Status: Open
* Priority: Normal
----------------------------------------
~~~
"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
~~~


---Files--------------------------------
0001-avoid-garbage-from-Symbol-to_s-in-interpolation.patch (6.32 KB)


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

[ruby-core:105749] Re: [Ruby master Feature#13715] [PATCH] avoid garbage from Symbol#to_s in interpolation

[Eric Wong]

"jeremyevans0 (Jeremy Evans)" <noreply@ruby-lang.org> wrote:
> @normalperson's patch: https://github.com/ruby/ruby/pull/5002,
> expanding the optimization to include nil/true/false/0-9.
> This is an across-the-board performance improvement, mostly by
> reducing the number of VM instructions from 6 to 1. From the
> benchmarks included in the patch, performance increase from
> the patch is roughly:

Thanks for bringing this up, again.  In Init_Numeric, you have:

	rb_str_freeze(rb_str_new_cstr(...));

Why not rb_fstring_cstr?

My original had a regression at loop_whileloop2, does that still
happen?  I didn't get the 6->1 instruction reduction in the
original, though, so the reduction bytecode alone seems worth it
in your version.

> It's definitely adds complexity, and I'm not sure how much of
> a difference it makes in a production application.  However,
> string interpolation is fairly common in most Ruby
> applications and libraries, and many interpolations will use
> symbols/nil/true/false/0-9, so I think it is worthy of
> consideration.

Agreed on all points.  I figured this would get lost in the
noise of typical real-world code that's overwhelmed by other
sources of garbage, so I didn't pursue it further.

However, the reduction in bytecode size nowadays seems like an
obvious win this time around.

[ruby-core:105754] [Ruby master Feature#13715] [PATCH] avoid garbage from Symbol#to_s in interpolation

[jeremyevans0 (Jeremy Evans)]

Issue #13715 has been updated by jeremyevans0 (Jeremy Evans).


normalperson (Eric Wong) wrote in #note-4:
> "jeremyevans0 (Jeremy Evans)" <noreply@ruby-lang.org> wrote:
>  > @normalperson's patch: https://github.com/ruby/ruby/pull/5002,
>  > expanding the optimization to include nil/true/false/0-9.
>  > This is an across-the-board performance improvement, mostly by
>  > reducing the number of VM instructions from 6 to 1. From the
>  > benchmarks included in the patch, performance increase from
>  > the patch is roughly:
>  
>  Thanks for bringing this up, again.  In Init_Numeric, you have:
>  
>  	rb_str_freeze(rb_str_new_cstr(...));
>  
>  Why not rb_fstring_cstr?

Other options recommended were `rb_str_new_literal` and `rb_fstring_literal`.  I went with `rb_fstring_literal`: https://github.com/ruby/ruby/commit/60d2206d9ebc1d7f301936f9d0c9bdb424a1feea.patch

>  My original had a regression at loop_whileloop2, does that still
>  happen?  I didn't get the 6->1 instruction reduction in the
>  original, though, so the reduction bytecode alone seems worth it
>  in your version.

When I was testing earlier versions of the patch, loop_whileloop2 didn't have a regression.  However, that was before the yjit merge and before I added more optimizations.  When running the tests now, I'm seeing a regression of about 11% for loop_whileloop2 in my environment, which seems like a definite red flag.  It would be good to get benchmark results from other people.

>  However, the reduction in bytecode size nowadays seems like an
>  obvious win this time around.

It definitely seems like a overall win for string interpolation.  However, if it makes the VM slower overall due to an increase in code size, it's probably not worth doing.  It may be possible to keep the single `tostring` instruction but reduce the number of cases it optimizes for to keep the code size down.  That may provide an overall string interpolation performance boost without a decrease in overall VM speed.  However, I'd like to get benchmarks from others before trying that approach.

----------------------------------------
Feature #13715: [PATCH] avoid garbage from Symbol#to_s in interpolation
https://bugs.ruby-lang.org/issues/13715#change-94261

* Author: normalperson (Eric Wong)
* Status: Open
* Priority: Normal
----------------------------------------
~~~
"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
~~~


---Files--------------------------------
0001-avoid-garbage-from-Symbol-to_s-in-interpolation.patch (6.32 KB)


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