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

[Fix #468] Fix false positives for Performance/BigDecimalWithNumericArgument #469

Conversation

koic
Copy link
Member

@koic koic commented Sep 17, 2024

Fixes #468.

This PR fixes false positives for Performance/BigDecimalWithNumericArgument when using float argument for BigDecimal.

In Ruby 3.1 and later, cases where numbers are faster than strings are limited to Integer. For Float, strings are still faster:

$ cat example.rb
require 'benchmark/ips'
require 'bigdecimal'
require 'bigdecimal/util'

Benchmark.ips do |x|
  x.report('float string')             { BigDecimal('4.2') }
  x.report('float with prec')          { BigDecimal(4.2, Float::DIG + 1) }
  x.report('to_d string without prec') { '4.2'.to_d }
  x.report('to_d float without prec')  { 4.2.to_d }
  x.report('to_d float with prec')     { 4.2.to_d(Float::DIG + 1) }
  x.compare!
end
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
Warming up --------------------------------------
        float string   246.214k i/100ms
     float with prec   173.880k i/100ms
to_d string without prec
                       255.950k i/100ms
to_d float without prec
                       181.033k i/100ms
to_d float with prec   151.091k i/100ms
Calculating -------------------------------------
        float string      2.418M (± 5.5%) i/s -     12.064M in   5.004969s
      float with prec      1.685M (± 4.0%) i/s -      8.520M in   5.064059s
to_d string without prec
                          2.460M (± 4.2%) i/s -     12.286M in   5.002392s
to_d float without prec
                          1.781M (± 6.5%) i/s -      8.871M in   5.007829s
to_d float with prec      1.584M (± 5.7%) i/s -      8.008M in   5.072184s

Comparison:
to_d string without prec:  2460462.7 i/s
        float string:  2418003.6 i/s - same-ish: difference falls within error
to_d float without prec:  1781070.6 i/s - 1.38x  slower
     float with prec:  1685372.9 i/s - 1.46x  slower
to_d float with prec:  1584419.5 i/s - 1.55x  slower

Before submitting the PR make sure the following are checked:

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

…NumericArgument`

Fixes rubocop#468.

This PR fixes false positives for `Performance/BigDecimalWithNumericArgument`
when using float argument for `BigDecimal`.

In Ruby 3.1 and later, cases where numbers are faster than strings are limited to `Integer`.
For `Float`, strings are still faster:

```console
$ cat example.rb
require 'benchmark/ips'
require 'bigdecimal'
require 'bigdecimal/util'

Benchmark.ips do |x|
  x.report('float string')             { BigDecimal('4.2') }
  x.report('float with prec')          { BigDecimal(4.2, Float::DIG + 1) }
  x.report('to_d string without prec') { '4.2'.to_d }
  x.report('to_d float without prec')  { 4.2.to_d }
  x.report('to_d float with prec')     { 4.2.to_d(Float::DIG + 1) }
  x.compare!
end
```

```console
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
Warming up --------------------------------------
        float string   246.214k i/100ms
     float with prec   173.880k i/100ms
to_d string without prec
                       255.950k i/100ms
to_d float without prec
                       181.033k i/100ms
to_d float with prec   151.091k i/100ms
Calculating -------------------------------------
        float string      2.418M (± 5.5%) i/s -     12.064M in   5.004969s
      float with prec      1.685M (± 4.0%) i/s -      8.520M in   5.064059s
to_d string without prec
                          2.460M (± 4.2%) i/s -     12.286M in   5.002392s
to_d float without prec
                          1.781M (± 6.5%) i/s -      8.871M in   5.007829s
to_d float with prec      1.584M (± 5.7%) i/s -      8.008M in   5.072184s

Comparison:
to_d string without prec:  2460462.7 i/s
        float string:  2418003.6 i/s - same-ish: difference falls within error
to_d float without prec:  1781070.6 i/s - 1.38x  slower
     float with prec:  1685372.9 i/s - 1.46x  slower
to_d float with prec:  1584419.5 i/s - 1.55x  slower
```
@koic koic merged commit 100cdfe into rubocop:master Sep 17, 2024
14 checks passed
@koic koic deleted the fix_false_positives_for_performance_big_decimal_with_numeric_argument_cop branch September 17, 2024 06:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Problem with BigDecimalWithNumericArgument cop
1 participant