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

Support in-place operations #191

Open
pca006132 opened this issue Feb 16, 2021 · 5 comments
Open

Support in-place operations #191

pca006132 opened this issue Feb 16, 2021 · 5 comments

Comments

@pca006132
Copy link

Currently, operations on BigInt such as add_assign is not done in-place, but with the following implementation:

    #[inline]
    fn add_assign(&mut self, other: &BigInt) {
        let n = mem::replace(self, BigInt::zero());
        *self = n + other;
    }

which would be costly for more complex operations, due to frequent allocation/deallocation.

We already have some in-place operations for BigUint such as add_assign, can we support this for BigInt as well? I'm not familiar with this so idk if it would be very difficult to do so.

@cuviper
Copy link
Member

cuviper commented Feb 16, 2021

BigInt::zero() doesn't require any allocation, so if that's your main concern, I think we should be fine. The goal here was to avoid duplicating the sign logic, which BigUint doesn't have to worry about. Do you have other specific examples of concern?

@pca006132
Copy link
Author

BigInt::zero() doesn't require any allocation, so if that's your main concern, I think we should be fine. The goal here was to avoid duplicating the sign logic, which BigUint doesn't have to worry about. Do you have other specific examples of concern?

No I don't mean BigInt::zero() requires allocation, but in-place operation can reduce allocation and improve performance. The example case is this:
image

which allocation occupies a significant time. Using += and *= cannot improve the performance.

@cuviper
Copy link
Member

cuviper commented Feb 18, 2021

What is the scale of the numbers being multiplied? If allocation is a significant portion of the calculation, I'm guessing they are relatively small values to begin with.

If one of the operands is only a single BigDigit, I think we could manage to do scalar multiplication in place. With some effort we could probably expand two digits to multiply in place too, but I would expect quickly diminishing returns.

@cuviper
Copy link
Member

cuviper commented Feb 18, 2021

Switching to smallvec could also help if even the result is small enough, depending on how many digits we keep local.

@pca006132
Copy link
Author

What is the scale of the numbers being multiplied? If allocation is a significant portion of the calculation, I'm guessing they are relatively small values to begin with.

Yes, I think so. The project is RustPython, bigint is used for storing all the numbers, so I think most of the numbers would be pretty small. And smallvec may also help.

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