support pre-allocation in BigInt ctor

[rfourquet]

The allows to initialize a BigInt with enough space to encode an n-bits integer (so this is an optimization compared to the function MPZ.realloc!, so as to avoid allocating twice).

This was initially implemented in #13815 and discussed there. It was suggested to make this functionality a method of sizehint!, but this has to be implemented in the inner constructor of BigInt, so I don't think it's possible. I don't mind the mecanism (I originally used a ::Val{:allocbits}, now ::typeof(sizehint!)), suggestions welcome!

[rfourquet]

What about creating a "BigInt" label? (or "bignum" to include BigFloat).

[rfourquet]

I realized there are already "rationals" and "complex" labels, so sureley a "bignums" label seems deserved!

[rfourquet]

This is a tiny change, I think only the API can be controversial: I would choose BigInt(Val{:uninitialized}) or something similar, but am fine with any. Can someone make a call, e.g. @JeffBezanson or @vtjnash who had already commented on the API in #13815 ?

[tkelman]

tests, docs?

[rfourquet]

My last post was confusing: currently this PR defines a method to reserve at creation time a space big enough to store later a number with n bits. An alternative is to be able to create an unitialized number, on which to call mpz.init2!.
I can add tests/docs when the API is decided upon.

[StefanKarpinski]

This is a bit of a strange API, IMO. How about BigInt(; sizehint::Int=0) instead?

[rfourquet]

It's strange and I would rather not use this API. I would prefer a Val{:unitit} or Val{:allocsize} argument, or a dedicated type, or even Type{Bool}. But I wanted to avoid a keyword argument because the goal of the PR is to make a small optimization, which becomes worse than the status quo with the overhead of keyword arguments. Currently b=BigInt() takes 50 ns on my machine and calling realloc(b, 200) from GMP on it costs 65ns; with a keyword arg, BigIng(sizehint=200) takes about 180ns, and BigInt() takes 120ns !; but BigInt(Val{:allocsize}, 200) takes about 50ns. Those numbers vary with the sizehint parameter (200 here), but always show that a keyword arg would be a no-go if optimization is sought after.

[tkelman]

keyword arguments are likely to get faster fairly soon, so we should avoid making too many ugly API choices as workarounds for them being slow now

[rfourquet]

Ok then I can wait for no-overhead keywords.

[rfourquet]

Now that keywords are fast, I reimplemented using the nbits keyword to control the bit length of the created BigInt (e.g. BigInt(nbits=128)). As an example, I updated the implementation of + for BigInt, which become 40% faster on my machine for 1-Limb bigints.
Please help bikesheding the nbits keyword name.

[rfourquet]

I will merge in a few days if no objection. The name of the keyword is rather inconsequential and remains for now an implementation detail.

[KristofferC]

If this is a performance change, should run Nanosoldier before merging.

[KristofferC]

@nanosoldier runbenchmarks(ALL, vs = ":master")

[nanosoldier]

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

[rfourquet]

There seems to be some real performance regressions.

[jebej]

Not sure how much it would matter but should we wait after #32102 before testing performance?

edit: it see it has been merged

[KristofferC]

Note, it's only been merged into the backport branch. But yes, it is true that the benchmarks might not be completely reliable if the bignum lib is built with bad optimizations.

[rfourquet]

I can't reproduce locally the regressions, let's try again.

@nanosoldier runbenchmarks(ALL, vs = ":master")

[nanosoldier]

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

[rfourquet]

In the two nanosoldier runs, I kept regressions wich are worse by more than 2% than what the tolerance allows, and the intersection contains 6 benchmarks, which are all unrelated to BigInt, so all shown regressions must be noise.