-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
avoid expensive runtime div in parse for common bases #29892
Conversation
@nanosoldier |
base/parse.jl
Outdated
m::T = div(typemax(T) - base + 1, base) | ||
# Special case the common cases of base being 10 or 16 to avoid expensive runtime div | ||
m::T = base == 10 ? div(typemax(T) - T(11), T(10)) : | ||
base == 16 ? div(typemax(T) - T(16), T(16)) : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these be - T(9)
and - T(15)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, should have ran the tests first heh.
17ccd38
to
d363f66
Compare
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan |
(cherry picked from commit 05180b3)
(cherry picked from commit 05180b3)
(cherry picked from commit 05180b3)
(cherry picked from commit 05180b3)
10 and 16 are common bases to parse to so let's avoid the "killer div" in those cases.
Benchmarks:
After
I have some thought on how this overflow handling can be done a bit better, but for now, might as well do the incremental thing.