From 40b7358e934e3b1f2cc7a664f97e5cc1393cbc77 Mon Sep 17 00:00:00 2001
From: John Hawthorn <john@hawthorn.email>
Date: Mon, 14 Dec 2020 17:34:31 -0800
Subject: Skip defined check in NODE_OP_ASGN_OR with ivar
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previously we would add code to check if an ivar was defined when using
`@foo ||= 123`, which was slower than `@foo || (@foo = 123)` when `@foo`
was already defined.

Recently 01b7d5acc702df22d306ae95f1a9c3096e63e624 made it so that
accessing an undefined variable no longer generates a warning, making
the defined check unnecessary and both statements exactly equal.

This commit avoids emitting the defined instruction when compiling
NODE_OP_ASGN_OR with a NODE_IVAR.

Before:

    $ ruby --dump=insn -e '@foo ||= 123'
    == disasm: #<ISeq:<main>@-e:1 (1,0)-(1,12)> (catch: FALSE)
    0000 putnil                                                           (   1)[Li]
    0001 defined                      instance-variable, :@foo, false
    0005 branchunless                 14
    0007 getinstancevariable          :@foo, <is:0>
    0010 dup
    0011 branchif                     20
    0013 pop
    0014 putobject                    123
    0016 dup
    0017 setinstancevariable          :@foo, <is:0>
    0020 leave

After:

    $ ./ruby --dump=insn -e '@foo ||= 123'
    == disasm: #<ISeq:<main>@-e:1 (1,0)-(1,12)> (catch: FALSE)
    0000 getinstancevariable                    :@foo, <is:0>             (   1)[Li]
    0003 dup
    0004 branchif                               13
    0006 pop
    0007 putobject                              123
    0009 dup
    0010 setinstancevariable                    :@foo, <is:0>
    0013 leave

This seems to be about 50% faster in this benchmark:

    require "benchmark/ips"

    class Foo
      def initialize
        @foo = nil
      end

      def test1
        @foo ||= 123
      end

      def test2
        @foo || (@foo = 123)
      end
    end

    FOO = Foo.new

    Benchmark.ips do |x|
      x.report("test1", "FOO.test1")
      x.report("test2", "FOO.test2")
    end

Before:

    $ ruby benchmark_ivar.rb
    Warming up --------------------------------------
                   test1     1.957M i/100ms
                   test2     3.125M i/100ms
    Calculating -------------------------------------
                   test1     20.030M (± 1.7%) i/s -    101.780M in   5.083040s
                   test2     31.227M (± 4.5%) i/s -    156.262M in   5.015936s

After:

    $ ./ruby benchmark_ivar.rb
    Warming up --------------------------------------
                   test1     3.205M i/100ms
                   test2     3.197M i/100ms
    Calculating -------------------------------------
                   test1     32.066M (± 1.1%) i/s -    163.440M in   5.097581s
                   test2     31.438M (± 4.9%) i/s -    159.860M in   5.098961s
---
 compile.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'compile.c')

diff --git a/compile.c b/compile.c
index 0525c3ed87..514fedcd3a 100644
--- a/compile.c
+++ b/compile.c
@@ -8024,7 +8024,7 @@ iseq_compile_each0(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *node, in
 	LABEL *lfin = NEW_LABEL(line);
 	LABEL *lassign;
 
-	if (nd_type(node) == NODE_OP_ASGN_OR) {
+	if (nd_type(node) == NODE_OP_ASGN_OR && nd_type(node->nd_head) != NODE_IVAR) {
 	    LABEL *lfinish[2];
 	    lfinish[0] = lfin;
 	    lfinish[1] = 0;
-- 
cgit v1.2.3