[PATCH 1/4] compile.c (opt_str_lit_1): hoist out of iseq_peephole_optimize

[PATCH 1/4] compile.c (opt_str_lit_1): hoist out of iseq_peephole_optimize

[Eric Wong]

---
 compile.c | 67 ++++++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 36 insertions(+), 31 deletions(-)

diff --git a/compile.c b/compile.c
index 205ff6a..f1123cc 100644
--- a/compile.c
+++ b/compile.c
@@ -1746,21 +1746,51 @@ new_recvinfo_for_arg_(rb_iseq_t *iseq, VALUE str,
 }
 
 /*
+ * optimize allocation:
+ *   hash["lit"] # hash lookups
+ *   str == "lit"
+ *   str != "lit"
+ *   str << "lit"
+ *   str + "lit"
+ *   str === "lit"
+ */
+static VALUE
+opt_str_lit_1(rb_iseq_t *iseq, VALUE str, rb_call_info_t *ci, INSN *list)
+{
+    enum ruby_optimized_method om;
+    VALUE c;
+
+    switch (ci->mid) {
+#define C(mid,klass) case mid: om = OM_##mid##__##klass; c = rb_c##klass; break
+      C(idAREF, Hash);
+      C(idEq, String);
+      C(idNeq, String);
+      C(idLTLT, String);
+      C(idPLUS, String);
+      C(idEqq, String);
+#undef C
+      default: return Qfalse;
+    }
+
+    return new_recvinfo_for_arg_(iseq, str, om, c, 0);
+}
+
+/*
  * optimize common calls which take two string literals:
  *   foo.sub(/../, "to")
  *   foo.sub!(/../, "to")
  *   foo.gsub(/../, "to")
  *   foo.gsub!(/../, "to")
- *   foo.tr(/../, "to")
- *   foo.tr!(/../, "to")
- *   foo.tr_s(/../, "to")
- *   foo.tr_s!(/../, "to")
+ *   foo.tr("from", "to")
+ *   foo.tr!("from", "to")
+ *   foo.tr_s("from", "to")
+ *   foo.tr_s!("from", "to")
  */
 static VALUE
 opt_str_lit_2(rb_iseq_t *iseq, VALUE str, rb_call_info_t *ci, INSN *list)
 {
     INSN *piobj;
-    enum ruby_optimized_method om = OM_LAST_;
+    enum ruby_optimized_method om;
 
     switch (ci->mid) {
 #define C(mid) case mid: om = OM_##mid##__String; break
@@ -1943,32 +1973,7 @@ iseq_peephole_optimize(rb_iseq_t *iseq, LINK_ELEMENT *list, const int do_tailcal
 		    }
 		    break;
 		  case 1:
-		    switch (ci->mid) {
-		      case idAREF:
-			/* optimize allocation: obj["lit"] */
-			ri = new_recvinfo_for_arg(iseq, str, idAREF, Hash, 0);
-			break;
-		      case idEq:
-			/* optimize allocation: obj == "lit" */
-			ri = new_recvinfo_for_arg(iseq, str, idEq, String, 0);
-			break;
-		      case idNeq:
-			/* optimize allocation: obj != "lit" */
-			ri = new_recvinfo_for_arg(iseq, str, idNeq, String, 0);
-			break;
-		      case idLTLT:
-			/* optimize allocation: obj << "lit" */
-			ri = new_recvinfo_for_arg(iseq, str, idLTLT, String, 0);
-			break;
-		      case idPLUS:
-			/* optimize allocation: obj + "lit" */
-			ri = new_recvinfo_for_arg(iseq, str, idPLUS, String, 0);
-			break;
-		      case idEqq:
-			/* optimize allocation: obj === "lit" */
-			ri = new_recvinfo_for_arg(iseq, str, idEqq, String, 0);
-			break;
-		    }
+		    ri = opt_str_lit_1(iseq, str, ci, (INSN *)list);
 		    break;
 		  case 2:
 		    ri = opt_str_lit_2(iseq, str, ci, (INSN *)list);
-- 
EW

[PATCH 2/4] test/ruby/test_optimization.rb (test_hash_aset_with): assert assignment

[Eric Wong]

---
 test/ruby/test_optimization.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/ruby/test_optimization.rb b/test/ruby/test_optimization.rb
index f991bea..40b1cba 100644
--- a/test/ruby/test_optimization.rb
+++ b/test/ruby/test_optimization.rb
@@ -164,7 +164,7 @@ class TestRubyOptimization < Test::Unit::TestCase
     assert_equal 1, h["foo"] = 1
     assert_redefine_method('Hash', '[]=', <<-end)
       h = {}
-      h["foo"] = 1
+      assert_equal 1, h["foo"] = 1, "assignment always returns value set"
       assert_nil h["foo"]
     end
   end
-- 
EW

[PATCH 3/4] test/ruby/test_optimization.rb: redefinition tests for string

[Eric Wong]

---
 test/ruby/test_optimization.rb | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/test/ruby/test_optimization.rb b/test/ruby/test_optimization.rb
index 40b1cba..2f9c0f6 100644
--- a/test/ruby/test_optimization.rb
+++ b/test/ruby/test_optimization.rb
@@ -116,6 +116,25 @@ class TestRubyOptimization < Test::Unit::TestCase
     assert_redefine_method('String', 'freeze', 'assert_nil "foo".freeze')
   end
 
+  def test_string_eq_neq
+    %w(== !=).each do |m|
+      assert_redefine_method('String', m, <<-end)
+        assert_equal :b, ("a" #{m} "b").to_sym
+        b = 'b'
+        assert_equal :b, ("a" #{m} b).to_sym
+        assert_equal :b, (b #{m} "b").to_sym
+      end
+    end
+  end
+
+  def test_string_ltlt
+    assert_equal "", "" << ""
+    assert_equal "x", "x" << ""
+    assert_equal "x", "" << "x"
+    assert_equal "ab", "a" << "b"
+    assert_redefine_method('String', '<<', 'assert_equal "b", "a" << "b"')
+  end
+
   def test_array_plus
     assert_equal [1,2], [1]+[2]
     assert_redefine_method('Array', '+', 'assert_equal [2], [1]+[2]')
-- 
EW

[PATCH 4/4] test/ruby/test_string.rb: cleanup allocation tests

[Eric Wong]

---
 test/ruby/test_string.rb | 76 ++++++++++++++++++++++++------------------------
 1 file changed, 38 insertions(+), 38 deletions(-)

diff --git a/test/ruby/test_string.rb b/test/ruby/test_string.rb
index 8d46764..3d82bb3 100644
--- a/test/ruby/test_string.rb
+++ b/test/ruby/test_string.rb
@@ -1910,9 +1910,9 @@ class TestString < Test::Unit::TestCase
 
   def test_literal_freeze
     require_compile_option(:peephole_optimization)
-    before = GC.stat(:total_allocated_objects)
-    5.times { "".freeze }
-    assert_equal before, GC.stat(:total_allocated_objects)
+    assert_no_new_allocations do
+      5.times { "".freeze }
+    end
   end
 
   class S2 < String
@@ -2296,14 +2296,14 @@ class TestString < Test::Unit::TestCase
     if @cls == String
       nr = 10
       recv = ""
-      before = GC.stat(:total_allocated_objects)
-      nr.times { recv << "constant" }
-      assert_equal before, GC.stat(:total_allocated_objects)
+      assert_no_new_allocations do
+        nr.times { recv << "constant" }
+      end
       assert_equal "constant" * nr, recv
 
-      before = GC.stat(:total_allocated_objects)
-      nr.times { "recv" << "constant" }
-      assert_equal before + nr, GC.stat(:total_allocated_objects)
+      assert_no_new_allocations("'lit' << 'lit' (LTLT)", nr) do
+        nr.times { "recv" << "constant" }
+      end
     end
   end
 
@@ -2386,48 +2386,48 @@ class TestString < Test::Unit::TestCase
 
       recv = "something"
       res = []
-      before = GC.stat(:total_allocated_objects)
-      nr.times { res << (recv == "constant") } # opt_streq1
-      nr.times { res << ("constant" == recv) } # opt_streq2
-      nr.times { res << ("something" != recv) } # 1st pass peephole
-      nr.times { res << ("constant" == recv) } # opt_streq2
-      nr.times { res << ("constant" === recv) } # opt_streqq2
-      nr.times { res << (recv != "something") }  # 2nd pass peephole
-      assert_equal before, GC.stat(:total_allocated_objects)
+      assert_no_new_allocations("false comparisons") do
+        nr.times { res << (recv == "constant") } # opt_streq1
+        nr.times { res << ("constant" == recv) } # opt_streq2
+        nr.times { res << ("something" != recv) } # 1st pass peephole
+        nr.times { res << ("constant" == recv) } # opt_streq2
+        nr.times { res << ("constant" === recv) } # opt_streqq2
+        nr.times { res << (recv != "something") }  # 2nd pass peephole
+      end
       assert_equal [ false ], res.uniq!
 
       res.clear
-      before = GC.stat(:total_allocated_objects)
-      nr.times { res << (recv == "something") } # opt_streq1
-      nr.times { res << ("something" == recv) } # opt_streq2
-      nr.times { res << ("something" === recv) } # opt_streqq2
-      nr.times { res << (recv === "something") } # opt_streqq2
-      nr.times { res << ("constant" != recv) } # 1st pass peephole
-      nr.times { res << (recv != "constant") } # 2nd pass peephole
-      nr.times { res << ("a" != "b") } # 1st pass peephole
-      nr.times { res << ("a" == "a") } # 1st pass peephole
-      nr.times { res << ("".size == 0) } # 2nd pass peephole
-      nr.times { res << ("".length == 0) } # 2nd pass peephole
-      assert_equal before, GC.stat(:total_allocated_objects)
+      assert_no_new_allocations("true comparisons") do
+        nr.times { res << (recv == "something") } # opt_streq1
+        nr.times { res << ("something" == recv) } # opt_streq2
+        nr.times { res << ("something" === recv) } # opt_streqq2
+        nr.times { res << (recv === "something") } # opt_streqq2
+        nr.times { res << ("constant" != recv) } # 1st pass peephole
+        nr.times { res << (recv != "constant") } # 2nd pass peephole
+        nr.times { res << ("a" != "b") } # 1st pass peephole
+        nr.times { res << ("a" == "a") } # 1st pass peephole
+        nr.times { res << ("".size == 0) } # 2nd pass peephole
+        nr.times { res << ("".length == 0) } # 2nd pass peephole
+      end
       assert_equal [ true ], res.uniq!
 
       # :+ optimizations
       res.clear
-      before = GC.stat(:total_allocated_objects)
-      nr.times { res << ("foo" + recv) }
-      assert_equal before + nr, GC.stat(:total_allocated_objects)
+      assert_no_new_allocations("'str' + (PLUS)", nr) do
+        nr.times { res << ("foo" + recv) }
+      end
       assert_equal [ "foosomething" ], res.uniq!
 
       res.clear
-      before = GC.stat(:total_allocated_objects)
-      nr.times { res << (recv + "foo") }
-      assert_equal before + nr, GC.stat(:total_allocated_objects)
+      assert_no_new_allocations("+ 'str' (PLUS)", nr) do
+        nr.times { res << (recv + "foo") }
+      end
       assert_equal [ "somethingfoo" ], res.uniq!
 
       res.clear
-      before = GC.stat(:total_allocated_objects)
-      nr.times { res << ('a' * 3) }
-      assert_equal before + nr, GC.stat(:total_allocated_objects)
+      assert_no_new_allocations("'str' * (MULT)", nr) do
+        nr.times { res << ('a' * 3) }
+      end
       assert_equal [ "aaa" ], res.uniq!
     end
   end
-- 
EW