-
Notifications
You must be signed in to change notification settings - Fork 77
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
Use bc math #43
Comments
Yeah, that's probably a good idea. I'll want to do some profiling to make sure we're not slowing things down too much, but I think this is probably an important improvement. |
@triplepoint I've done some benchmarks out of curiosity, and for fewer than ~1,000 operations it seems to not really make a difference — either will take < 1ms. BCMath is a lot slower if you're wanting to do tens of thousands of operations, but we're talking ~20ms vs ~2ms, which seems reasonable to me within the intended purpose of this library. There is also the issue that BCMath is not included by default with PHP (except on windows for some reason), but I think it seems reasonable to require it in order to be able to get accurate results out and avoid floating-point errors. If that sounds reasonable to you, I'd be happy to go through and change everything to BCMath. |
What would it take to enable BCMath if available and the default if not? |
hey @triplepoint this would be really useful, is this on the roadmap? |
I've created a patch that can be installed via composer-patches. The patch defaults the precision to the value set by php. This was a quick-and-dirty fix, so I'm hoping others can review and improve the efficiency. |
http://php.net/manual/en/book.bc.php
The text was updated successfully, but these errors were encountered: