Feature #13355: [PATCH] compile.c: optimize literal String range in case/when dispatch

• Status: Closed
• Assignee: -
• Target version: -

[normalperson (Eric Wong)]

This seems obvious, and manages to pass existing tests.
Earlier, I found myself writing code like:

case RUBY_VERSION
when '2.0.0',
     '2.1.0'..'2.1.9',
     '2.2.0'..'2.2.6',
     '2.3.0'..'2.3.3',
     '2.4.0'
  # install workaround
else
  puts "good!"
end

And figured other people might write similar code somewhere.

This is similar in spirit to opt_case_dispatch as the literal
Range here is guaranteed to be immutable when used for
checkmatch.

Normal range literals with non-frozen strings are actually
mutable, as Range#begin and Range#end exposes the strings to
modification. So those Range objects cannot be frozen without
breaking compatibility, but Ranges in case/when dispatch can be
frozen at compile time.

• compile.c (iseq_peephole_optimize): persistent Range creation
  when String literals are used as beginning and end of range
  when used for case/when dispatch.

[normalperson (Eric Wong)]

[e-mail] wrote:

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

This is similar in spirit to opt_case_dispatch as the literal
Range here is guaranteed to be immutable when used for
checkmatch.

Along with the current behavior of using putobject instead of
putstring for literal strings, when there are non-literals.

Anyways, this only puts extra code into compile.c (not insns.def),
so it has less negative impact at runtime.

I will commit this in a few days if no response.

• Status changed from Open to Closed

Applied in changeset trunk|r58233.

---

compile.c: optimize literal String range in case/when dispatch

This is similar in spirit to opt_case_dispatch as the literal
Range here is guaranteed to be immutable when used for
checkmatch.

Normal range literals with non-frozen strings are actually
mutable, as Range#begin and Range#end exposes the strings to
modification. So those Range objects cannot be frozen without
breaking compatibility, but Ranges in case/when dispatch can be
frozen at compile time.

• compile.c (iseq_peephole_optimize): persistent Range creation
  when String literals are used as beginning and end of range
  when used for case/when dispatch.
  [ruby-core:80290] [Feature #13355]

[normalperson (Eric Wong)]

Eric Wong [e-mail] wrote:

[e-mail] wrote:

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

I will commit this in a few days if no response.

Caught during self review:

• Use more descriptive variable names (avoid potential "send" conflict)

• s/iseq_add_mark_object_compile_time/iseq_add_mark_object/
  since the range lifetime is tied to the overall iseq lifetime.

Will commit with following changes (after tests run...)

--- a/compile.c
+++ b/compile.c
@@ -2162,12 +2162,12 @@ iseq_peephole_optimize(rb_iseq_t *iseq, LINK_ELEMENT *list, const int do_tailcal
IS_INSN_ID(end, putstring) &&
(beg = (INSN *)get_prev_insn(end)) != 0 &&
IS_INSN_ID(beg, putstring)) {

•  VALUE sbeg = OPERAND_AT(beg, 0);
  

•  VALUE send = OPERAND_AT(end, 0);
  

•  VALUE str_beg = OPERAND_AT(beg, 0);
  

•  VALUE str_end = OPERAND_AT(end, 0);
   int excl = FIX2INT(OPERAND_AT(range, 0));
  

•  VALUE lit_range = rb_range_new(sbeg, send, excl);
  

•  VALUE lit_range = rb_range_new(str_beg, str_end, excl);
  

•  iseq_add_mark_object_compile_time(iseq, lit_range);
  

•  iseq_add_mark_object(iseq, lit_range);
   REMOVE_ELEM(&beg->link);
   REMOVE_ELEM(&end->link);
   range->insn_id = BIN(putobject);
  

[normalperson (Eric Wong)]

Eric Wong [e-mail] wrote:

• s/iseq_add_mark_object_compile_time/iseq_add_mark_object/
  since the range lifetime is tied to the overall iseq lifetime.

Brrr.. I was wrong the second time :x

iseq_add_mark_object is called in iseq_set_sequence later on,
so we only need to call iseq_add_mark_object_compile_time
in iseq_peephole_optimize.