Improve String#casecmp? and Symbol#casecmp? performance

## Background `String#casecmp?` method was suggested to RuboCop Performance to use as a fast method. rubocop/rubocop-performance#100 However `String#casecmp?` is always slower than `String#casecmp("arg").zero?`. ## Summary This PR improves `String#casecmp?` and `Symbol#casecmp?` performance when comparing ASCII characters. OTOH, in the case of Non-ASCII characters, performance is slightly degraded by added condition. And I found ruby#1668 which is essentially the same. ## Benchmark The following is a benchmark case for `String#casecmp?` with similar cases. ```ruby require 'benchmark/ips' puts '* ASCII' Benchmark.ips do |x| x.report('upcase') { 'String'.upcase == 'string' } x.report('downcase') { 'String'.downcase == 'string' } x.report('casecmp.zero?') { 'String'.casecmp('string').zero? } x.report('casecmp?') { 'String'.casecmp?('string') } x.compare! end puts '* Non-ASCII' Benchmark.ips do |x| x.report('upcase') { 'äö '.upcase == ('ÄÖÜ') } x.report('downcase') { 'äö '.downcase == ('ÄÖÜ') } x.report('casecmp.zero?') { 'äö '.casecmp('ÄÖÜ').zero? } x.report('casecmp?') { 'äö '.casecmp?('ÄÖÜ') } x.compare! end ``` ### Before ```console * ASCII Warming up -------------------------------------- upcase 200.428k i/100ms downcase 197.503k i/100ms casecmp.zero? 216.244k i/100ms casecmp? 171.257k i/100ms Calculating ------------------------------------- upcase 4.326M (± 2.9%) i/s - 21.646M in 5.008511s downcase 4.350M (± 2.3%) i/s - 21.923M in 5.042694s casecmp.zero? 4.998M (± 3.7%) i/s - 25.084M in 5.025253s casecmp? 3.090M (± 2.9%) i/s - 15.584M in 5.047497s Comparison: casecmp.zero?: 4998357.4 i/s downcase: 4349766.0 i/s - 1.15x slower upcase: 4325765.3 i/s - 1.16x slower casecmp?: 3090194.2 i/s - 1.62x slower * Non-ASCII Warming up -------------------------------------- upcase 137.352k i/100ms downcase 136.735k i/100ms casecmp.zero? 206.341k i/100ms casecmp? 96.326k i/100ms Calculating ------------------------------------- upcase 2.215M (± 2.7%) i/s - 11.126M in 5.027582s downcase 2.199M (± 2.5%) i/s - 11.076M in 5.038781s casecmp.zero? 4.709M (± 1.8%) i/s - 23.729M in 5.041362s casecmp? 1.315M (± 1.6%) i/s - 6.646M in 5.054479s Comparison: casecmp.zero?: 4708502.7 i/s upcase: 2214590.5 i/s - 2.13x slower downcase: 2199478.9 i/s - 2.14x slower casecmp?: 1315326.8 i/s - 3.58x slower ``` ### After `casecmp?` performance for ASCII characters will be improved. ```console * ASCII Warming up -------------------------------------- upcase 206.368k i/100ms downcase 205.971k i/100ms casecmp.zero? 225.636k i/100ms casecmp? 232.030k i/100ms Calculating ------------------------------------- upcase 4.337M (± 1.3%) i/s - 21.875M in 5.045187s downcase 4.297M (± 1.5%) i/s - 21.627M in 5.033750s casecmp.zero? 5.240M (± 1.4%) i/s - 26.399M in 5.038613s casecmp? 5.548M (± 1.5%) i/s - 27.844M in 5.019895s Comparison: casecmp?: 5547954.3 i/s casecmp.zero?: 5240466.7 i/s - 1.06x slower upcase: 4336522.3 i/s - 1.28x slower downcase: 4297370.8 i/s - 1.29x slower ### Non-ASCII Warming up -------------------------------------- upcase 141.841k i/100ms downcase 141.233k i/100ms casecmp.zero? 215.401k i/100ms casecmp? 94.625k i/100ms Calculating ------------------------------------- upcase 2.238M (± 2.1%) i/s - 11.205M in 5.010247s downcase 2.243M (± 1.3%) i/s - 11.299M in 5.038967s casecmp.zero? 4.611M (± 2.6%) i/s - 23.048M in 5.001970s casecmp? 1.255M (± 2.4%) i/s - 6.340M in 5.055611s Comparison: casecmp.zero?: 4610838.3 i/s downcase: 2242621.9 i/s - 2.06x slower upcase: 2237553.1 i/s - 2.06x slower casecmp?: 1254739.1 i/s - 3.67x slower ``` `Symbol#casecmp?` have similar results. ## Other Information This is an off-topic for this PR. I inform RuboCop Performance that `String#casecmp` and `String#casecmp?` behave differently when using Non-ASCII characters. ```ruby 'äöü'.casecmp('ÄÖÜ').zero? #=> false 'äöü'.casecmp?('ÄÖÜ') #=> true ```
koic · Feb 29, 2020 · 50b4dad · 50b4dad
1 parent 9d6d531
commit 50b4dad
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 11 deletions.
diff --git a/spec/ruby/core/string/casecmp_spec.rb b/spec/ruby/core/string/casecmp_spec.rb
@@ -145,8 +145,18 @@
     "abc".casecmp?(other).should == true
   end
 
-  it "returns nil if incompatible encodings" do
-    "あれ".casecmp?("れ".encode(Encoding::EUC_JP)).should be_nil
+  describe "when incompatible encodings" do
+    describe "for ASCII characters" do
+      it "returns nil" do
+        "abc".casecmp?("abc".encode(Encoding::UTF_16BE)).should be_nil
+      end
+    end
+
+    describe "for non-ASCII characters" do
+      it "returns nil" do
+        "あれ".casecmp?("れ".encode(Encoding::EUC_JP)).should be_nil
+      end
+    end
   end
 
   describe 'for UNICODE characters' do

diff --git a/string.c b/string.c
@@ -3480,19 +3480,27 @@ rb_str_casecmp_p(VALUE str1, VALUE str2)
 static VALUE
 str_casecmp_p(VALUE str1, VALUE str2)
 {
-    rb_encoding *enc;
-    VALUE folded_str1, folded_str2;
-    VALUE fold_opt = sym_fold;
+    if (rb_enc_str_asciionly_p(str1)) {
+        VALUE ret = str_casecmp(str1, str2);
+        if (NIL_P(ret)) return Qnil;
 
-    enc = rb_enc_compatible(str1, str2);
-    if (!enc) {
-	return Qnil;
+        return FIXNUM_ZERO_P(ret) ? Qtrue : Qfalse;
     }
+    else {
+        rb_encoding *enc;
+        VALUE folded_str1, folded_str2;
+        VALUE fold_opt = sym_fold;
 
-    folded_str1 = rb_str_downcase(1, &fold_opt, str1);
-    folded_str2 = rb_str_downcase(1, &fold_opt, str2);
+        enc = rb_enc_compatible(str1, str2);
+        if (!enc) {
+            return Qnil;
+        }
 
-    return rb_str_eql(folded_str1, folded_str2);
+        folded_str1 = rb_str_downcase(1, &fold_opt, str1);
+        folded_str2 = rb_str_downcase(1, &fold_opt, str2);
+
+        return rb_str_eql(folded_str1, folded_str2);
+    }
 }
 
 static long