num-bigint icon indicating copy to clipboard operation
num-bigint copied to clipboard

Support in-place operations

Open pca006132 opened this issue 4 years ago • 5 comments

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.

pca006132 avatar Feb 16 '21 08:02 pca006132

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?

cuviper avatar Feb 16 '21 18:02 cuviper

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.

pca006132 avatar Feb 17 '21 01:02 pca006132

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 avatar Feb 18 '21 19:02 cuviper

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

cuviper avatar Feb 18 '21 19:02 cuviper

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.

pca006132 avatar Feb 19 '21 00:02 pca006132