Performance issue with Contracts gem about truffleruby HOT 10 CLOSED

Thanks this is definitely something we will enjoy looking into in the next few days!

from truffleruby.

It's only slower on the String case, though, is that right? It seems an order of magnitude faster already for Integer?

from truffleruby.

That's because the order in which overloads are defined affects their performance. The Integer overload is the first that was defined, so the lookup for the matching method ends earlier.

from truffleruby.

Right, but the order in which the overloads is defined is the same for MRI and TruffleRuby isn't it? So with the same overloads, defined in the same order, MRI runs the integer case at 154630.2 i/s and TruffleRuby at 3899698.2 i/s. That's 25x faster.

I see that the string case is terrible though - it's so bad that I presume this is deoptimising each time it runs.

from truffleruby.

Yes, the first overload defined runs faster on TruffleRuby no matter what types you restrict it by. The issue is probably related to the way Contracts checks each overload until it finds the right one.

from truffleruby.

Here's an example with more bite.
Worst case: MRI: 4004.4 i/s, TruffleRuby: 56.8 i/s
The slowest (Overload 4) is resolved in this way, involving 4 lookups that raise a total of 6 exceptions:

Integer 🔴 -> String 🔴 -> Float 🔴 -> Array<Float> 💚
Integer 🔴 -> String 🔴 -> Float 💚
Integer 🔴 -> String 💚
Integer 💚

from truffleruby.

The first thing I've done is to run the specs on TruffleRuby - there's no point running faster if it's not correct - and they all passed first time which is great!

from truffleruby.

Ah so this uses exceptions as part of normal control flow? Yes that not going to be fast at the moment and we knew that.

Like most language implementors, we would not recommend using exceptions for control flow in most cases. They're slower than other forms of control flow, they get slower the deeper you are in a stack trace, they generate garbage and so on. They're particularly slow for us at the moment as generating a stack trace means re-materialising objects we had escape analysed and not allocated. The same thing applies to some extent to JRuby.

  i.report('plain exception') do
    begin
      raise Exception.new
    rescue Exception
    end
  end

  i.report('lib exception') do
    begin
      raise ParamContractError.new(nil, nil)
    rescue ParamContractError
    end
  end

  i.report('fails') do
    begin
      FOO.a = 'string'
    rescue ParamContractError
    end
  end

  i.report('single') do
    FOO.a = 1
  end

  i.report('multi first') do
    FOO.b = 1
  end

  i.report('multi second') do
    FOO.b = '1'
  end

MRI

     plain exception:   829703.9 i/s
       lib exception:   629946.0 i/s - 1.32x  slower
         multi first:   350623.9 i/s - 2.37x  slower
              single:   331375.6 i/s - 2.50x  slower
        multi second:    44215.9 i/s - 18.76x  slower
               fails:    19977.1 i/s - 41.53x  slower

TruffleRuby

              single:  4198545.2 i/s
         multi first:  3944998.6 i/s - same-ish: difference falls within error
       lib exception:     2141.1 i/s - 1960.89x  slower
     plain exception:     2138.3 i/s - 1963.53x  slower
        multi second:      771.5 i/s - 5442.10x  slower
               fails:      490.5 i/s - 8559.40x  slower

JRuby

         multi first:   343511.1 i/s
              single:   342523.3 i/s - same-ish: difference falls within error
     plain exception:    11319.7 i/s - 30.35x  slower
       lib exception:    11203.4 i/s - 30.66x  slower
        multi second:     5736.7 i/s - 59.88x  slower
               fails:     3171.7 i/s - 108.30x  slower

I'm not really sure what to do here - exceptions are unlikely to ever be fast, on highly optimising implementations of Ruby and they're slower than you might want even on MRI. I would recommend to use something other than exceptions to the maintainers even if I didn't work on TruffleRuby.

Could failing overloads return a special value rather than throwing an exception? A parameter to say to do that could be part of Contracts::CallWith.call_with perhaps? Could failure_callback be used somehow?

from truffleruby.

I modified contracts.ruby to return exceptions, rather than raise them, in the case of overloaded methods, until they all fail, when I do throw an exception. This seems to basically work - I get only 2 spec failures - so could perhaps be used in practice.

diff --git a/lib/contracts/call_with.rb b/lib/contracts/call_with.rb
index 8c66d22..481a72b 100644
--- a/lib/contracts/call_with.rb
+++ b/lib/contracts/call_with.rb
@@ -1,6 +1,10 @@
 module Contracts
   module CallWith
     def call_with(this, *args, &blk)
+      call_with_inner(false, this, *args, &blk)
+    end
+    
+    def call_with_inner(returns, this, *args, &blk)
       args << blk if blk
 
       # Explicitly append blk=nil if nil != Proc contract violation anticipated
@@ -16,14 +20,16 @@ module Contracts
         validator = @args_validators[i]
 
         unless validator && validator[arg]
-          return unless Contract.failure_callback(:arg => arg,
-                                                  :contract => contract,
-                                                  :class => klass,
-                                                  :method => method,
-                                                  :contracts => self,
-                                                  :arg_pos => i+1,
-                                                  :total_args => args.size,
-                                                  :return_value => false)
+          data = {:arg => arg,
+            :contract => contract,
+            :class => klass,
+            :method => method,
+            :contracts => self,
+            :arg_pos => i+1,
+            :total_args => args.size,
+            :return_value => false}
+          return ParamContractError.new('as return value', data) if returns
+          return unless Contract.failure_callback(data)
         end
 
         if contract.is_a?(Contracts::Func) && blk && !nil_block_appended
diff --git a/lib/contracts/method_handler.rb b/lib/contracts/method_handler.rb
index f47926b..262f8c1 100644
--- a/lib/contracts/method_handler.rb
+++ b/lib/contracts/method_handler.rb
@@ -133,10 +133,10 @@ module Contracts
         until success
           decorated_method = decorated_methods[i]
           i += 1
-          begin
-            success = true
-            result = decorated_method.call_with(self, *args, &blk)
-          rescue expected_error => error
+          success = true
+          result = decorated_method.call_with_inner(true, self, *args, &blk)
+          if result.is_a?(ParamContractError)
+            error = result
             success = false
             unless decorated_methods[i]
               begin

This is faster for all implementations of Ruby, not just TruffleRuby, and allows the optimising implementations like JRuby and TruffleRuby to work.

MRI is now 3 times faster than before for hitting the second overload (multi second).

TruffleRuby is then an order of magnitude faster than that. JRuby is 1.3 times as fast as MRI. Rubinius doesn't do great - it's 7 times slower than MRI.

Look at create exception and fails to see the bottom line - TruffleRuby can create exceptions five orders of magnitude faster than it can raise them.

MRI

    create exception:  4263013.2 i/s
     plain exception:   828593.9 i/s - 5.14x  slower
       lib exception:   641660.3 i/s - 6.64x  slower
              single:   347303.5 i/s - 12.27x  slower
         multi first:   340321.1 i/s - 12.53x  slower
        multi second:   141677.7 i/s - 30.09x  slower
               fails:    28008.2 i/s - 152.21x  slower

JRuby

    create exception: 10842419.2 i/s
         multi first:   323535.5 i/s - 33.51x  slower
              single:   322905.5 i/s - 33.58x  slower
        multi second:   185076.5 i/s - 58.58x  slower
     plain exception:    11091.4 i/s - 977.55x  slower
       lib exception:    11057.8 i/s - 980.52x  slower
               fails:     6438.1 i/s - 1684.09x  slower

TruffleRuby

    create exception: 133346926.6 i/s
              single:  4464367.6 i/s - 29.87x  slower
         multi first:  4244310.3 i/s - 31.42x  slower
        multi second:  2088957.5 i/s - 63.83x  slower
     plain exception:     1610.3 i/s - 82806.72x  slower
       lib exception:     1608.7 i/s - 82890.63x  slower
               fails:     1335.2 i/s - 99867.99x  slower

Rubinius

    create exception:  1612196.1 i/s
     plain exception:   185078.1 i/s - 8.71x  slower
       lib exception:   171645.6 i/s - 9.39x  slower
              single:    47119.2 i/s - 34.22x  slower
         multi first:    46742.3 i/s - 34.49x  slower
        multi second:    20594.0 i/s - 78.28x  slower
               fails:     3607.3 i/s - 446.93x  slower

from truffleruby.

Ah, so that's what it was. Before opening the issue I tried raising and rescuing an exception in the report block for the first overload to see if it was related to that, but I used raise 'error'; rescue; end and the performance hit was much less severe than in your examples (it went from 3_827_387 i/s to 2_723_166 i/s).

Could failing overloads return a special value rather than throwing an exception? A parameter to say to do that could be part of Contracts::CallWith.call_with perhaps? Could failure_callback be used somehow?

I'm not the author of the gem so I have no sway over its implementation. I'll have to do what you did. It will not be the first time I privately monkeypatch a gem to remove superfluous and wasteful stuff.

Perhaps this should be mentioned in /doc/user/compatibility.md under "Features with very low performance".

Thanks for playing detective with me 😄

from truffleruby.