From: Eric Wong <normalperson@...>
Date: 2014-11-18T12:12:12+00:00
Subject: [ruby-core:66343] wondering about %I(symbol array behavior)

I expected: %I(a b)
to generate the same bytecode as: %i(a b)
Because of %W and %w are equivalent when there is no interpolation,
and also because :"literal string with colon"  generates a single
putobject instruction

In other words, %I(a b) bytecode seems suboptimal because it
is designed for %I(a#{foo} b#{bar}) use cases:

== disasm: <RubyVM::InstructionSequence:<compiled>@<compiled>>==========
0000 trace            1                                               (   1)
0002 putobject        "a"
0004 concatstrings    1
0006 opt_send_simple  <callinfo!mid:intern, argc:0, ARGS_SKIP>
0008 putobject        "b"
0010 concatstrings    1
0012 opt_send_simple  <callinfo!mid:intern, argc:0, ARGS_SKIP>
0014 newarray         2
0016 leave

I started working on the following patch to avoid generating the
concatstrings instructions (and swap the putobject calls
with putstring):

--- a/compile.c
+++ b/compile.c
@@ -2303,6 +2303,14 @@ compile_dstr(rb_iseq_t *iseq, LINK_ANCHOR *ret, NODE * node)
 {
     int cnt;
     compile_dstr_fragments(iseq, ret, node, &cnt);
+    if (cnt == 1) {
+	INSN *last = (INSN *)ret->last;
+	if (last->insn_id == BIN(putobject) &&
+	    RB_TYPE_P(OPERAND_AT(last, 0), T_STRING)) {
+	    last->insn_id = BIN(putstring);
+	    return COMPILE_OK;
+	}
+    }
     ADD_INSN1(ret, nd_line(node), concatstrings, INT2FIX(cnt));
     return COMPILE_OK;
 }
~~~
However, I also like to remove the String#intern calls entirely.
This is a user-visible behavior change, but I think it valid to be
consistent with non-array :"literal string" use, so
I also came up with the following while writing this email:

--- a/compile.c
+++ b/compile.c
@@ -5374,7 +5374,18 @@ iseq_compile_each(rb_iseq_t *iseq, LINK_ANCHOR *ret, NODE * node, int poped)
       case NODE_DSYM:{
 	compile_dstr(iseq, ret, node);
 	if (!poped) {
-	    ADD_SEND(ret, line, idIntern, INT2FIX(0));
+	    INSN *last = (INSN *)ret->last;
+
+	    if (last->insn_id == BIN(putstring)) {
+		VALUE obj = OPERAND_AT(last, 0);
+
+		obj = ID2SYM(rb_intern_str(obj));
+		OPERAND_AT(last, 0) = obj;
+		last->insn_id = BIN(putobject);
+	    }
+	    else {
+		ADD_SEND(ret, line, idIntern, INT2FIX(0));
+	    }
 	}
 	else {
 	    ADD_INSN(ret, line, pop);
~~~

Which now results in:

== disasm: <RubyVM::InstructionSequence:<compiled>@<compiled>>==========
0000 trace            1                                               (   1)
0002 putobject        :a
0004 putobject        :b
0006 newarray         2
0008 leave

Much better than before, and further (not-yet-done) optimization can
morph the above into code which is identical to %i(a b):

== disasm: <RubyVM::InstructionSequence:<compiled>@<compiled>>==========
0000 trace            1                                               (   1)
0002 duparray         [:a, :b]
0004 leave

Now I wonder if there's a better way to accomplish this
(perhaps in parse.y instead of compile.c).
But it's way past my bedtime :x