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.

Sign up for GitHub

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

Jump to bottom

apd: optimize Quo, remove per-digit long division #114

Merged
merged 5 commits into from
Jan 31, 2022
Merged

apd: optimize Quo, remove per-digit long division #114

merged 5 commits into from
Jan 31, 2022

Commits on Jan 31, 2022

  1. apd: use IsUint64 instead of BitLen in Decimal.Reduce 

    Code cleanup. Should not change behavior, but makes the code's purpose
clearer and may be slightly faster.
    @nvanbenschoten
    nvanbenschoten committed Jan 31, 2022
    Configuration menu
    Copy the full SHA
    da6c77f View commit details
    Browse the repository at this point in the history

  2. apd: optimize Quo, remove per-digit long-division 

    Fixes cockroachdb#98.

This commit speeds up `Context.Quo`, replacing its per-digit long
division with a call to `BigInt.QuoRem`. This avoids per-digit
iteration. It also allows `Context.Quo` to benefit further from the
inline fast-path of `BigInt.QuoRem` for small coefficient values that
fit in a uint64.

This is a partial revert of 1eddda3, at least in spirit. Before that
commit, `Context.Quo` did try to use `big.Int.Quo`. Unfortunately, it
was inaccurate for certain values because it was not scaling the
coefficients correctly before dividing. It tried to compensate for this
by using a very large precision (`c.Precision*2 + 8`), but this was
insufficient on its own because it was not taking the size of the values
into account. So the commit abandoned that approach.

However, that commit, which was based on the description given on the
GDA site, did demonstrate how to scale the coefficients in a way that
would permit this kind of division. Specifically, it began adjusting the
operands so that the coefficient of the dividend was greater than or
equal to the coefficient of the divisor and was also less than ten times
the coefficient of the divisor.

This commit uses the coefficient adjustment introduced in 1eddda3 to
revive the call into `BigInt.QuoRem`. With the two operands adjusted, it
now is possible to scale the coefficient of the dividend by the desired
precision and then perform a direct division on the operands.
    @nvanbenschoten
    nvanbenschoten committed Jan 31, 2022
    Configuration menu
    Copy the full SHA
    909da8f View commit details
    Browse the repository at this point in the history

  3. apd: remove looping when adjusting coefficients in Quo 

    This commit replaces the looping in Quo when adjusting coefficients to
be integer divisible with a fixed set of operations that have the same
effect.
    @nvanbenschoten
    nvanbenschoten committed Jan 31, 2022
    Configuration menu
    Copy the full SHA
    ae1b836 View commit details
    Browse the repository at this point in the history

  4. apd: unskip some GDA tests 

    These all pass now. I don't know why.
    @nvanbenschoten
    nvanbenschoten committed Jan 31, 2022
    Configuration menu
    Copy the full SHA
    c46294f View commit details
    Browse the repository at this point in the history

  5. apd: remove precision limit in Quo 

    Now that we removed the long division, we don't directly iterate per
digit in Quo, so we can remove the precision limit.
    @nvanbenschoten
    nvanbenschoten committed Jan 31, 2022
    Configuration menu
    Copy the full SHA
    d607123 View commit details
    Browse the repository at this point in the history