Optimizing BigInt for small integers

[smarter]

@denisrosset

• Is there a thing as “too much test code” for BigInt ? I’ll to err on the cautious side due to amount of changes this introduces.

The more tests the better :).

@denisrosset

• What would be the roadblocks to have this merged? Does it introduce risk for too small a benefit? Do I need to demonstrate performance benefits to have the merge approved in 2.14?

If the motivation for the change is performance then I think having benchmarks demonstrating the improvements is a reasonable requirement to get it merged.

@denisrosset

• I have one question about the design. At the final step, BigInt will have two implementations, one holding a Long , one holding a BigInteger . Currently, Spire’s implementation has the invariant that the BigInteger instance must not hold an integer that’s a valid Long . That enables a few additional optimizations but forces all instance creations to go through a size test, and adds a bit of mental overhead when reading the code. What do you think?

I think that’s reasonable, benchmarks demonstrating these additional optimizations would be nice.

@denisrosset

• After step B), the modifications would be merged against 2.14. There is no branch named 2.14 yet, right?

Yes, for now you can create your PR against 2.13.x, you can then retarget it to 2.14.x once that branch has been created.