Avoid use of exceptions to detect invalid floats #43106
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Tagging @p8 whose idea this was. 🙌 |
flavorjones
force-pushed
the
flavorjones-activesupport-number-converter-valid-float
branch
from
fd1e2da to
2b831a1
Compare
|
Note that I've updated this PR to make a similar modification to ActionView::Helpers::NumberHelper. |
Use Kernel::Float(..., exceptions:false) instead of a rescue block in ActionView::Helpers::NumberHelper and ActiveSupport::NumberHelper::NumberConverter to slightly improve performance. Also remove documentation that incorrectly states ActiveSupport::NumberHelper supports the `raise:` option.
flavorjones
force-pushed
the
flavorjones-activesupport-number-converter-valid-float
branch
from
2b831a1 to
900ce92
Compare
flavorjones
changed the title
Kernel::Float(..., exception: false) in NumberConverter#valid_float?
|
Nice! 😄🙌 |
byroot
reviewed
Aug 26, 2021
| Float(number) | ||
| rescue ArgumentError, TypeError | ||
| false | ||
| Float(number, exception: false) |
There was a problem hiding this comment.
Hum, the return will be nil instead of false, but I don't think it's worth adding a || false.
|
Thanks, also the perf impact is dependant on the stack size, so that 3X could be way larger in real word scenarios. |
flavorjones
deleted the
flavorjones-activesupport-number-converter-valid-float
branch
ghiculescu
reviewed
Dec 13, 2021
| rescue ArgumentError, TypeError | ||
| raise InvalidNumberError, number if raise_error | ||
| result = Float(number, exception: false) | ||
| raise InvalidNumberError, number if result.nil? && raise_error |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Improves the performance of ActiveSupport::NumberHelper and ActionView::Helpers::NumberHelper formatters by avoiding the use of exceptions as flow control.
Other Information
Note that
Float(..., exception: false)is only available as of Ruby 2.7.Benchmarks
I benchmarked this by calling
ActiveSupport::NumberHelper.number_to_roundedwith:Here are the results:
The new signature is comparable or slightly faster for Floats and valid strings, and much wow faster on invalid strings.