Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

decimal performance #5650

Closed
stevenjwolfman opened this Issue Mar 12, 2019 · 4 comments

Comments

Projects
None yet
2 participants
@stevenjwolfman
Copy link

stevenjwolfman commented Mar 12, 2019

Environment

Provide at least:

  • jruby 9.2.5.0 (2.5.0) 2018-12-06 6d5a228 Java HotSpot(TM) 64-Bit Server VM 25.121-b13 on 1.8.0_121-b13 +jit [darwin-x86_64]
  • Darwin ip-192-168-30-11.ec2.internal 18.0.0 Darwin Kernel Version 18.0.0: Wed Aug 22 20:13:40 PDT 2018; root:xnu-4903.201.2~1/RELEASE_X86_64 x86_64

When implementing the Haversine formula (http://stackoverflow.com/a/12969617/3443587), if I converted latitude and longitude to a decimal before running it, it took more than 150 times longer than if I left it as a float. This seems excessive and an opportunity for tuning.

This slow performance was not found in JRuby 1.7

For just multiplying simple numbers
irb(main):002:0> Benchmark.measure{1000.times{5.05.0}}
#<Benchmark::Tms:0x41cec7b0 @stime=0.0, @Label="", @CsTime=0.0, @real=0.000554911996005103, @ToTal=0.0, @cutime=0.0, @utime=0.0>
irb(main):003:0> Benchmark.measure{1000.times{5.0.to_d
5.0.to_d}}
#<Benchmark::Tms:0x6773c073 @stime=0.009999999999999787, @Label="", @CsTime=0.0, @real=0.015367278996563982, @ToTal=0.04999999999999183, @cutime=0.0, @utime=0.03999999999999204>

@kares

This comment has been minimized.

Copy link
Member

kares commented Mar 21, 2019

that measure is really dump simple - you should usually measure longer times with JRuby.
Decimal#* seems to be on par with outperform MRI - likely to have the same performance under 1.7 :

require 'benchmark'

TIMES = (ENV['TIMES'] || 100_000_000).to_i

Benchmark.bmbm do |bm|
  bm.report("int") { int = 5  ; TIMES.times { int * int } }
  bm.report("flt") { flt = 5.0; TIMES.times { flt * flt } }
  bm.report("dec") { dec = BigDecimal('5.0'); TIMES.times { dec * dec } }
end

MRI 2.5 :

Rehearsal ---------------------------------------
int   3.836824   0.000000   3.836824 (  3.836857)
flt   4.268661   0.000000   4.268661 (  4.268683)
dec  36.192001   0.000000  36.192001 ( 36.192149)
----------------------------- total: 44.297486sec

          user     system      total        real
int   3.771479   0.000000   3.771479 (  3.771489)
flt   4.282776   0.000000   4.282776 (  4.282799)
dec  38.846768   0.000000  38.846768 ( 38.847088)

JRuby 9.2 :

int   2.660000   0.010000   2.670000 (  2.308014)
flt   3.030000   0.120000   3.150000 (  2.819985)
dec   9.580000   0.050000   9.630000 (  9.320902)
----------------------------- total: 15.450000sec

          user     system      total        real
int   2.040000   0.000000   2.040000 (  2.035778)
flt   2.320000   0.010000   2.330000 (  2.303343)
dec   9.490000   0.020000   9.510000 (  9.439375)
@kares

This comment has been minimized.

Copy link
Member

kares commented Mar 21, 2019

if I converted latitude and longitude to a decimal before running it, it took more than 150 times longer than if I left it as a float.

so before trying to guess what exactly have you been testing, maybe you did not run that long enough?
Decimal in 9.2 should perform better than in 1.7, in general, however if you're seeing smt different its might be worth looking into (might turn out not that much of a Decimal performance regression).

@kares

This comment has been minimized.

Copy link
Member

kares commented Mar 21, 2019

for the distance calculation, smt definitely is slower than needs to be :

require 'bigdecimal'

def distance loc1, loc2
  rad_per_deg = Math::PI/180  # PI / 180
  rkm = 6371                  # Earth radius in kilometers
  rm = rkm * 1000             # Radius in meters

  dlat_rad = (loc2[0]-loc1[0]) * rad_per_deg  # Delta, converted to rad
  dlon_rad = (loc2[1]-loc1[1]) * rad_per_deg

  lat1_rad, lon1_rad = loc1.map {|i| i * rad_per_deg }
  lat2_rad, lon2_rad = loc2.map {|i| i * rad_per_deg }

  a = Math.sin(dlat_rad/2)**2 + Math.cos(lat1_rad) * Math.cos(lat2_rad) * Math.sin(dlon_rad/2)**2
  c = 2 * Math::atan2(Math::sqrt(a), Math::sqrt(1-a))

  rm * c # Delta in meters
end

loc1 = [46.3625, 15.114444]
loc2 = [46.055556, 14.508333]
puts distance(loc1, loc2)

# => 57794.35510874037

loc1 = [BigDecimal('46.3625'), BigDecimal('15.114444')]
loc2 = [BigDecimal('46.055556'), BigDecimal('14.508333')]
puts distance(loc1, loc2)


require 'benchmark'

TIMES = (ENV['TIMES'] || 100_000).to_i

Benchmark.bmbm do |bm|
  bm.report("distance flt") {
    loc1 = [46.3625, 15.114444]
    loc2 = [46.055556, 14.508333]
    TIMES.times { distance(loc1, loc2) }
  }
  bm.report("distance dec") {
    loc1 = [BigDecimal('46.3625'), BigDecimal('15.114444')]
    loc2 = [BigDecimal('46.055556'), BigDecimal('14.508333')]
    TIMES.times { distance(loc1, loc2) }
  }
end

MRI 2.5.3 :

Rehearsal ------------------------------------------------
distance flt   0.141682   0.000201   0.141883 (  0.141905)
distance dec   2.591159   0.000000   2.591159 (  2.591163)
--------------------------------------- total: 2.733042sec

                   user     system      total        real
distance flt   0.133691   0.000000   0.133691 (  0.133693)
distance dec   2.575590   0.000000   2.575590 (  2.575601)

JRuby 9.2.6 :

Rehearsal ------------------------------------------------
distance flt   1.900000   0.040000   1.940000 (  0.552498)
distance dec  11.920000   0.220000  12.140000 ( 10.123179)
-------------------------------------- total: 14.080000sec

                   user     system      total        real
distance flt   0.150000   0.000000   0.150000 (  0.123651)
distance dec  10.080000   0.060000  10.140000 ( 10.049417)
@kares

This comment has been minimized.

Copy link
Member

kares commented Mar 21, 2019

turns out the problem is BigDecimal's division - we tent to spent most time there.
its been problematic previously esp. calculating precision (JRuby is likely too precise compared to MRI).

@kares kares added this to the JRuby 9.2.7.0 milestone Apr 2, 2019

@kares kares closed this Apr 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.