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

DoubleToStringConverter::ToPrecision has poor (worse than sprintf) performance for a large set of numbers #83

Open
troelstm opened this issue Nov 23, 2018 · 1 comment
Assignees

Comments

@troelstm
Copy link

The problem lies in DigitGenCounted (in fast-dtoa.cc):
If the fraction is 0 after the first iteration generating the integrals, then the second iteration will be skipped, and less than the requested number of digits (will typically) have been generated, resulting in the method return false, which then leads to a fallback to bignum based conversion.

It's trivial to add a check for the fraction being exactly zero after the first iteration, and then padding with 0 up to the requested number of digits.

What I however can't (yet) answer with 100% certainty is whether this could generate wrong results for some input? (I don't really think so).

The performance issue is seen e.g. for any number in form N*10^M

@floitsch floitsch self-assigned this Nov 26, 2018
@floitsch
Copy link
Collaborator

floitsch commented Dec 3, 2018

I would need to check more, but I believe that the check is necessary, because the function has to assume that the input w is rounded and has an error of 1 ulp.
I'm guessing that there are cases where the created w is within the "correct" range and thus doesn't have any rounding error. However, there is currently no way to figure this out inside the DigitGenCounted function.
It might be interesting to check whether the incoming number fits into a 64bit integer, though. At that point a simple integer "printing" would be faster and simpler (and would never fall back to the big-int solution).

I'm currently very busy, and don't expect to find the time to implement this myself. I could, however, give pointers on where to change the library.

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

No branches or pull requests

2 participants