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

Comparisons with large exponents are extremely slow #109

Open
DanielBauman88 opened this issue Jul 24, 2023 · 2 comments
Open

Comparisons with large exponents are extremely slow #109

DanielBauman88 opened this issue Jul 24, 2023 · 2 comments

Comments

@DanielBauman88
Copy link
Contributor

        let a = BigDecimal::from_str("1e99999999999999").unwrap();
        let zero = BigDecimal::zero();
        let bool = a == zero;

I haven't spent a lot of time looking into what's happening here, but it looks like the lib is getting stuck in ten_to_the

I'd expect BigDecimal to function in reasonable time, or to throw errors if there are values it can't process efficiently. I wouldn't expect it to hang on processing valid values.

I'm running into some of these changes by using BigDecimal to parse some JSON inputs which aren't under my control - which I'm going to work on avoiding in future.

@DanielBauman88 DanielBauman88 changed the title Comparisons are extremely slow Comparisons with large exponents are extremely slow Jul 24, 2023
@akubera
Copy link
Owner

akubera commented Jul 25, 2023

Looking into this raises (in my opinion) another issue: when debugging "big" decimals, the debugger will attempt to stringify the entire decimal.

This is related to #108, where lack of exponential syntax might panic.

Regardless: this can be fixed without the planned reimplementation. I think I can knock that out quickly.

@akubera
Copy link
Owner

akubera commented Jul 25, 2023

Going for a double on this one:

I was hoping to optimize ten_to_the as well as PartialEq::eq.

As there often is, I have two competing algorithms. I used criterion to benchmark different values of n (in 10^n) and it looks like there's a clear point to switch over:

image

By my calculations (on my computer) it's about n=1410 (i.e. 10^1410).

Unfortunately, they're both obviously climbing rapidly, so I'm going to check a few more things.

The primary issue is fixed, I will squash commits and push soon. It requires allocations to compare decimals digit-by-digit, which will be a lot more efficient than the old code (written ~7 years ago!).

This was fun, thanks.

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