Remove a source of O(n^2) running time in bigints.

[pnkfelix]

::num::bigint, Remove a source of O(n^2) running time in fn shr_bits.

I'll cut to the chase: On my laptop, this brings the running time on
pidigits 2000 (from src/test/bench/shootout-pidigits.rs) from this:

% time ./pidigits 2000 > /dev/null

real    0m7.695s
user    0m7.690s
sys 0m0.005s

to this:

% time ./pidigits 2000 > /dev/null

real    0m0.322s
user    0m0.318s
sys 0m0.004s

The previous code was building up a vector by repeatedly making a
fresh copy for each element that was unshifted onto the front,
yielding quadratic running time. This fixes that by building up the
vector in reverse order (pushing elements onto the end) and then
reversing it.

(Another option would be to build up a zero-initialized vector of the
desired length and then installing all of the shifted result elements
into their target index, but this was easier to hack up quickly, and
yields the desired asymptotic improvement. I have been thinking of
adding a vec::from_fn_rev to handle this case, maybe I will try that
this weekend.)

[huonw]

Cool; there's another instance of almost exactly the same pattern on line ~440, could you do this change there as well?

[huonw]

r=me if you do that and add a #[bench] e.g.

let n = BigUint::from_uint(1) << 1000;
bh.iter(|| {
    let mut m = n.clone();
    for _ in range(0, 100) {
        m = m >> 1;
    }
})

(Adjust the 100 & 1000 constants as necessary to get a reasonable benchmark.)

[pnkfelix]

@huonw I'm not sure the two cases are analogous.

In the case that my commit is addressing, the generated vector is of O(n) length, where n is the length of the input. Thus building via repeated unshifting is taking O(n^2) time.

In the case on line ~440 (within fn div_estimate(a: &BigUint, b: &BigUint, n: uint), we are not iterating over the entire length of a nor b. We are only iterating over a slice of length n. Now, that would still be troubling to me and I would normally put in your change, except... am I crazy, or is n always either 1 or 2 in every call to div_estimate?

Given that, I think we should just leave the loop on line ~440 alone. If my hypothesis above is correct, then it should be replaced with code to just handle the two cases of n in {1, 2}, right?

---

I will however add the #[bench] as you suggest.

[huonw]

am I crazy, or is n always either 1 or 2 in every call to div_estimate?

Good catch! It certainly looks like it.

[nikomatsakis]

On Wed, Feb 12, 2014 at 02:30:49AM -0800, Felix S Klock II wrote:

I have been thinking of
adding a vec::from_fn_rev to handle this case, maybe I will try that
this weekend.)

We have to be very careful about the cleanups here in case of
failure. For now we can get away with zeroing; maybe not forever,
though I imagine we could use a custom destructor scheme.

[nikomatsakis]

On Wed, Feb 12, 2014 at 02:30:49AM -0800, Felix S Klock II wrote:

I have been thinking of adding a vec::from_fn_rev to handle this
case, maybe I will try that this weekend.)

Oh, I guess you can leave memory at the front uninitialized, and then
in the destructor you just shift all elements from the right to the
end and set the length. In the case of a successful run, this will be
zero elements. All set.

[brson]

Nice find, @pnkfelix