Project

General

Profile

Bug #16243

Updated by Eregon (Benoit Daloze) over 4 years ago

This comes from a discussion on a PR on Sidekiq: https://github.com/mperham/sidekiq/pull/4303 

 This benchmark: 
 ```ruby 
 # frozen_string_literal: true 
 require "benchmark/ips" 

 def deep_dup_case(obj) 
   case obj 
   when Integer, Float, TrueClass, FalseClass, NilClass 
     obj 
   when String 
     obj.dup 
   when Array 
     obj.map { |e| deep_dup_case(e) } 
   when Hash 
     duped = obj.dup 
     duped.each_pair do |key, value| 
       duped[key] = deep_dup_case(value) 
     end 
   else 
     obj.dup 
   end 
 end 

 def deep_dup_if(obj) 
   if Integer === obj || Float === obj || TrueClass === obj || FalseClass === obj || NilClass === obj 
     obj 
   elsif String === obj 
     obj.dup 
   elsif Array === obj 
     obj.map { |e| deep_dup_if(e) } 
   elsif Hash === obj 
     duped = obj.dup 
     duped.each_pair do |key, value| 
       duped[key] = deep_dup_if(value) 
     end 
     duped 
   else 
     obj.dup 
   end 
 end 


 obj = { "class" => "FooWorker", "args" => [1, 2, 3, "foobar"], "jid" => "123987123" } 

 Benchmark.ips do |x| 
   x.report("deep_dup_case") do 
     deep_dup_case(obj) 
   end 

   x.report("deep_dup_if") do 
     deep_dup_if(obj) 
   end 

   x.compare! 
 end 
 ``` 

 gives in MRI 2.6.5: 
 ``` 
 Warming up -------------------------------------- 
        deep_dup_case      37.767k i/100ms 
          deep_dup_if      41.802k i/100ms 
 Calculating ------------------------------------- 
        deep_dup_case      408.046k (± 0.9%) i/s -        2.077M in     5.090997s 
          deep_dup_if      456.657k (± 0.9%) i/s -        2.299M in     5.035040s 

 Comparison: 
          deep_dup_if:     456657.4 i/s 
        deep_dup_case:     408046.1 i/s - 1.12x    slower 
 ``` 

 It seems @tenderlovemaking @tenderlove already noticed this a while ago due to missing inline caches for `case`, but the performance bug seems to still remain: 
 https://youtu.be/b77V0rkr5rk?t=1442 

 Do you think this could be fixed in MRI? 
 AFAIK both JRuby and TruffleRuby support inline cached `===` calls in `case/when`. 

 I think the code with `case` is more idiomatic and readable and should be preferred to `if`, and this performance issue makes that less clear.

Back