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

Performance regression in packDecimal/numDecimalDigits #2

Open
wrengr opened this issue Jun 11, 2015 · 1 comment
Open

Performance regression in packDecimal/numDecimalDigits #2

wrengr opened this issue Jun 11, 2015 · 1 comment
Assignees
Labels

Comments

@wrengr
Copy link
Owner

wrengr commented Jun 11, 2015

The current version of packDecimal shows a 3x slowdown compared to the version in the benchmark. This is due to the n0 > fromIntegral (maxBound :: Word64) guard in numDecimalDigits. This must be corrected. a 1.2x slowdown may be acceptable, but 3x is not.

@wrengr
Copy link
Owner Author

wrengr commented May 31, 2018

On further investigation, the slowdown is almost surely due to taking the slow-path when we shouldn't, rather than due to the guard itself introducing slowness; that is, the guard is returning true when it shouldn't. The fromIntegral function has modular-arithmetic semantics for all the built-in Int/Word types, and this is causing things to go haywire.

For example, when a ~ Int64 we have that fromIntegral(maxBound::Word64) == -1, which in turn means n0 > -1 will always return true (since we've already filtered out the negative numbers), thus we'll always take the slow-path— even though in truth every single non-negative 'Int64' can be perfectly well embedded into Word64, so there's no actual risk of the fast-path breaking.

The easiest fix is to change the guard to toInteger n0 > toInteger (maxBound::Word64). If the guard itself seems to be introducing the slowness, then we can do some bit-bashing to speed that up.

@wrengr wrengr added the bug label May 31, 2018
@wrengr wrengr self-assigned this May 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant