skip raise_unsupported_coercion for nil / performance issue

[dspaeth-faber]

When a value is nil Coercible::UnsupportedCoercion is raised. This raise is a real expensive operation. When I simply create a virtus model 1000.times I get the following profile result

# with virtus
%self      total      self      wait     child     calls  name
 31.30      0.952     0.493     0.000     0.460     4000   Kernel#loop 
 16.98      0.312     0.267     0.000     0.045   132000  *RubyVM::DebugInspector#frame_binding 
  9.92      1.362     0.156     0.000     1.205     8000  *Coercible::Coercer::Object#raise_unsupported_coercion 

# without virtus
%self      total      self      wait     child     calls  name
 41.31      0.002     0.001     0.000     0.001     1000   Class#new 
 38.43      0.003     0.001     0.000     0.002        1   Integer#times 
 19.14      0.001     0.001     0.000     0.000     1000   Model2#initialize 

for this test program:

require 'ruby-prof'

class Model
  include ::Virtus.model

  attribute :code, String
  attribute :reference, String
  attribute :state, String
  attribute :created_at, DateTime
  attribute :id, Integer

  include Equalizer.new(:code, :reference)
end

RubyProf.start

1000.times { Model.new({}) }

result = RubyProf.stop
printer = RubyProf::FlatPrinter.new(result)
printer.print(STDOUT)

class Model2
  attr_accessor :code
  attr_accessor :reference
  attr_accessor :state
  attr_accessor :created_at
  attr_accessor :id

  include Equalizer.new(:code, :reference)

  def initialize(*args)
  end

end

puts "\n"*5

RubyProf.start

1000.times { Model2.new({}) }

result = RubyProf.stop
printer = RubyProf::FlatPrinter.new(result)
printer.print(STDOUT)

It would be great if there is an shortcut for nil or performance increase.

Partly duplicate of #15

[gravityrail]

:+1: for this, we're seeing a doubling of CPU usage due to the number of exceptions being raised an caught. If you just returned value instead of raising it would make an enormous performance difference for no functional difference.

[gravityrail]

FWIW, we're using this monkeypatch to solve the issue in our Rails app:

# config/initializers/coercible_monkeypatch.rb
# significant performance improvement by not raising exceptions when we can't coerce a value
# it's trapped and returned as "value" by Virtus anyway! 50% CPU improvement
require 'coercible'
module Coercible
  class Coercer
    class Object
      def coerce_with_method(value, method, ref_method)
        value.respond_to?(method) ? value.public_send(method) : value #was: raise_unsupported_coercion(value, ref_method)
      end
    end
  end
end

[kml]

That's my patch. It's same problem but solved at Virtus level https://gist.github.com/kml/da4f7cf70008986b3ba5

[brgaulin]

👍 This issue just bit my team as well =( We were on JRuby which makes those very painful. Saved us 3 seconds, ~2k exceptions

[ivoanjo]

Also bit in https://github.com/jruby/jruby/issues/4540

[ivoanjo]

I spent some time looking at this today and it's definitely not trivial to optimize this for virtus while not possibly breaking other clients.

In particular, this exception is part of coercible's interface, so just removing it may be ok for virtus use only, but not ok if some other gem relies on coercible.

The same thing happens on virtus' side: the coercers can be configurable through virtus so just assuming that you don't have a custom coercer means that we could be stepping on someone else's toes.

I'm kinda lost. And since virtus and coercible are not really in active development I don't think this will ever get a good fix. Probably we'll just have to look somewhere else, or just use a hack and pray that it never has a bad interaction.

[ivoanjo]

Ok so I've been revisiting this issue and have a workaround that doesn't change the semantics of the library, and that manages to avoid monkey patching.

If someone is still interested in this, see https://github.com/solnic/coercible/pull/23 :)