xirr icon indicating copy to clipboard operation
xirr copied to clipboard

Remove ruby inline and c function call

Open milus opened this issue 3 years ago • 1 comments

Use pure ruby instead of Inline feature. Since Ruby math functions are already implemented in C it does not impact on performance. In C extention BigDecmial is casted to double. Ruby's Float internally is also implemented as double, so casting BigDecimal to Float in ruby has the same effect without compromising on performance

Benchmark:

require 'date'
require 'benchmark'
require 'xirr'

def test_xirr
  cf = Xirr::Cashflow.new
  cf << Xirr::Transaction.new(-1000,  date: Date.parse('2014-01-01'))
  cf << Xirr::Transaction.new(-2000,  date: Date.parse('2014-03-01'))
  cf << Xirr::Transaction.new( 4500, date: Date.parse('2015-12-01'))
  cf.xirr
end

n = ENV.fetch('N', 1_000).to_i

module Xirr
  module Base
    def xnpv(rate)
      cf.inject(0) do |sum, t|
        sum + xnpv_impl(rate, t.amount, periods_from_start(t.date))
      end
    end
  end
end

Benchmark.bm do |x|
  x.report('Inline implementation [double]') do
    module Xirr
      module Base
        def xnpv_impl(*args)
          xnpv_c(*args)
        end
      end
    end

    n.times { test_xirr }
  end

  x.report('Ruby implementation [Float]') do
    module Xirr
      module Base
        def xnpv_impl(rate, amount, period)
          amount / (1+rate.to_f) ** period
        end
      end
    end

    n.times { test_xirr }
  end

  x.report('Ruby implementation [BigDecimal]') do
    module Xirr
      module Base
        def xnpv_impl(rate, amount, period)
          amount / (1+rate) ** period
        end
      end
    end

    n.times { test_xirr }
  end
end

Benchmark result:

# $ N=10000 ruby -Ilib benchmark.rb
#        user     system      total        real
# Inline implementation [double]  1.187576   0.001224   1.188800 (  1.189479)
# Ruby implementation [Float]  1.171843   0.001200   1.173043 (  1.173044)
# Ruby implementation [BigDecimal] 23.502473   0.019631  23.522104 ( 23.524580)

milus avatar Dec 22 '21 08:12 milus

Should we remove BigDecimal entirely as well?

tubedude avatar Feb 25 '24 13:02 tubedude