[boost] [xint] Boost.XInt review

Subject: [boost] [xint] Boost.XInt review
From: Anders Dalvander (boost_at_[hidden])
Date: 2011-03-11 14:56:40

Dear all,

This is my review of the proposed XInt library. Please excuse my 
blunt/concise language.

I see great potential for an arbitrary-precision integer library and I 
applaud Chad for what he has accomplished over the last six iterations 
of the library.

But with the XInt library I see too many outstanding issues regarding 
both the design and the implementation which I believe need to be 
addressed before the library should be accepted into Boost. Thus I have 
to vote NO at this time.

>          - What is your evaluation of the design?

Data structures should be separated from algorithms. Algorithms should 
work on concepts.

Certain algorithms are on the border or outside the domain of the 
library, such as random_by_size, is_probable_prime, random_prime, and 
the secure option.

I see a problem with Not-a-Number as it breaks one of the basic 
properties of algebra, namely the additive inverse property, i + (-i) = 
0. Wrapping using boost::optional would perhaps be a better choice.

Signed-Zero provides nothing for the XInt library.

Providing Signed-Zero and Not-a-Number but not Positive-Infinity and 
Negative-Infinity seems backwards.

In this day and age the design must be thread-safe.

I'm not confident that COW semantics is a wise design choice.

bool arguments should be replaced with enums.

>          - What is your evaluation of the implementation?

BOOST_THROW_EXCEPTION should be used for throwing exceptions.

Duplicated/dead code for throwing and non-throwing classes should be 
eliminated.

Even though I somewhat question the validity of the most recent COW vs 
Move performance testing, the results are actually dissatisfying for 
COW. The reason I question the validity is due that Move isn't 
implemented for the internal classes.

Anyway, comparing the worst and the best case the 8Kbit integer test 
shows 3.2% performance improvement, while the 2Kbit integer test shows 
6.3% performance improvement. Changing the multiplication algorithm to 
one with better complexity would give much better overall performance 
improvements.

>          - What is your evaluation of the documentation?

Documentation should use BoostBook. Some features are hard to find, such 
as the different supported options and how they interact.

>          - What is your evaluation of the potential usefulness of the library?

The potential usefulness for an arbitrary-precision integer library is huge.

>          - Did you try to use the library?  With what compiler?  Did you have any
>          problems?

Yes, I used the library with VS2010. Without any issues, except rather 
long compile times. Probably my computer is to blame.

>          - How much effort did you put into your evaluation? A glance? A quick
>          reading? In-depth study?

Thorough study of documentation. Testing library for about 5 hours. 
Reading code for another 2 hours. Running different benchmarks. 
Following the discussion with high interest since first preliminary 
review in March last year.

>          - Are you knowledgeable about the problem domain?

No.

Best regards,
Anders Dalvander

-- 
WWFSMD?