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

Implement BigDecimal's missing rounding modes #10798

Merged
merged 5 commits into from
Jun 17, 2021

Conversation

HertzDevil
Copy link
Contributor

@HertzDevil HertzDevil commented Jun 8, 2021

Resolves #5714. Resolves #10599. Supersedes #7126.

The main difference with #7126 is this PR depends only on the previously unused BigInt#unsafe_truncated_divmod method and not Number#digits, which means an Array allocation can be avoided:

require "benchmark"

N = ENV["N"].to_i

struct BigDecimal
  # implementation from #7126
  def round_old(digits : Int = 0, *, mode : RoundingMode)
    ...
    n_digits = @value.abs.digits.reverse! # small optimization i added
    ...
  end
end

macro test(method, mode)
  (N // 1000).times do
    x = Random.rand(-100.0..100.0).to_big_d
    1000.times { x.{{ method.id }}(2, mode: {{ mode }}) }
  end
end

Benchmark.ips do |b|
  b.report("ties_even new") { test round, :ties_even }
  b.report("ties_even old") { test round_old, :ties_even }
  b.report("ties_away new") { test round, :ties_away }
  b.report("ties_away old") { test round_old, :ties_away }
  b.report("to_positive new") { test round, :to_positive }
  b.report("to_positive old") { test round_old, :to_positive }
  b.report("to_negative new") { test round, :to_negative }
  b.report("to_negative old") { test round_old, :to_negative }
  b.report("to_zero new") { test round, :to_zero }
  b.report("to_zero old") { test round_old, :to_zero }
end
$ N=500000 bin/crystal run --release test.cr
  ties_even new   1.86  (537.30ms) (± 1.45%)  279MB/op   1.15× slower
  ties_even old   1.78  (561.14ms) (± 1.70%)  420MB/op   1.20× slower
  ties_away new   1.86  (537.10ms) (± 1.46%)  279MB/op   1.15× slower
  ties_away old   1.74  (575.14ms) (± 3.60%)  423MB/op   1.23× slower
to_positive new   1.97  (506.82ms) (± 3.40%)  268MB/op   1.08× slower
to_positive old   1.76  (568.56ms) (± 3.21%)  419MB/op   1.21× slower
to_negative new   2.04  (491.06ms) (± 2.75%)  260MB/op   1.05× slower
to_negative old   1.79  (557.82ms) (± 2.55%)  419MB/op   1.19× slower
    to_zero new   2.13  (468.69ms) (± 2.54%)  246MB/op        fastest
    to_zero old   2.05  (488.43ms) (± 1.89%)  378MB/op   1.04× slower

Similar benchmarks for BigDecimals with known low scales:

macro test(method, mode)
  (N // 1000).times do
    x = BigDecimal.new(Random.rand(20000) - 10000) / 100.0
    1000.times { x.{{ method.id }}(1, mode: {{ mode }}) }
  end
end
$ N=500000 bin/crystal run --release test.cr
  ties_even new   2.73  (366.45ms) (± 2.33%)  182MB/op   1.18× slower
  ties_even old   2.61  (383.56ms) (± 2.36%)  288MB/op   1.24× slower
  ties_away new   2.67  (375.16ms) (± 3.51%)  184MB/op   1.21× slower
  ties_away old   2.57  (388.99ms) (± 2.38%)  290MB/op   1.26× slower
to_positive new   2.88  (346.71ms) (± 5.91%)  173MB/op   1.12× slower
to_positive old   2.60  (385.17ms) (± 3.27%)  287MB/op   1.25× slower
to_negative new   2.92  (342.06ms) (± 6.60%)  166MB/op   1.11× slower
to_negative old   2.56  (390.17ms) (± 3.06%)  290MB/op   1.26× slower
    to_zero new   3.23  (309.36ms) (± 5.18%)  152MB/op        fastest
    to_zero old   3.11  (321.83ms) (± 5.22%)  250MB/op   1.04× slower

The existing rounding modes are also reimplemented on top of this PR. This dramatically improves #ceil's performance:

struct BigDecimal
  # implementations before this PR
  def ceil_old; ...; end
  def floor_old; ...; end
  def trunc_old; ...; end
end

macro test(method)
  (N // 1000).times do
    x = BigDecimal.new(Random.rand(20000) - 10000) / 100.0
    1000.times { x.{{ method.id }} }
  end
end

Benchmark.ips do |b|
  b.report("ceil new") { test :ceil }
  b.report("ceil old") { test :ceil_old }
  b.report("floor new") { test :floor }
  b.report("floor old") { test :floor_old }
  b.report("trunc new") { test :trunc }
  b.report("trunc old") { test :trunc_old }
end
$ N=500000 bin/crystal run --release test.cr
 ceil new   8.44  (118.43ms) (± 5.35%)  67.7MB/op   1.45× slower
 ceil old   3.64  (274.99ms) (± 3.26%)   128MB/op   3.37× slower
floor new   9.40  (106.38ms) (± 4.79%)  60.1MB/op   1.30× slower
floor old   7.17  (139.48ms) (± 4.10%)  67.6MB/op   1.71× slower
trunc new  12.24  ( 81.70ms) (± 5.93%)  45.2MB/op        fastest
trunc old   4.24  (235.79ms) (± 3.41%)   113MB/op   2.89× slower

Copy link
Member

@straight-shoota straight-shoota left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That round_impl using unsafe_truncated_divmod looks really neat 👍

src/big/big_decimal.cr Outdated Show resolved Hide resolved
src/big/big_decimal.cr Outdated Show resolved Hide resolved
src/big/big_decimal.cr Outdated Show resolved Hide resolved
spec/std/big/big_decimal_spec.cr Outdated Show resolved Hide resolved
@straight-shoota straight-shoota added kind:feature topic:stdlib:numeric kind:regression Something that used to correctly work but no longer works labels Jun 8, 2021
@HertzDevil
Copy link
Contributor Author

Bump

@asterite asterite added this to the 1.1.0 milestone Jun 17, 2021
@asterite asterite merged commit c490f18 into crystal-lang:master Jun 17, 2021
@HertzDevil HertzDevil deleted the feature/bigdecimal-round branch June 18, 2021 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:feature kind:regression Something that used to correctly work but no longer works topic:stdlib:numeric
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BigDecimal#round regression Implement rounding modes for BigDecimal
3 participants