[boost] [xint] quick review

Subject: [boost] [xint] quick review
From: Domagoj Saric (dsaritz_at_[hidden])
Date: 2011-03-11 23:00:49

Apologies for lack of proper review structure, I'm working with last bits of 
free time to post before the deadline...

IMO, the proposed XInt, as can be downloaded from the vault, should NOT be
accepted into Boost.

Rationale:

The reasons were mostly already covered by other boost.devel posters and
pretty much all fall into the categories of a too coupled/inflexible and
inefficient design.

My primary concern is lack of support for true, efficient, no-fail-guarantee 
fixed-size integers:
the provided fixed-size support still uses dynamic memory allocation (which
can fail and requires/introduces EH code) and the nothrow policy does not in 
fact
disable the use of exceptions (it only wraps and 'eats' them with try-catch
blocks...which is just plain inefficient...this is like implementing malloc
in terms of new instead of the other way around). For such low level 
operations, as fixed-sized integer arithmetic, it is quite reasonable to 
expect the implementation to be able to work even without linking to the CRT 
(i.e. no use of keywords new, virtual or throw)...

This might at first be viewed as a possibly 'conditional yes' vote but since 
Chad has repeatedly said that he considers fixed-sized integers to be 
'second class citizens' of his library I vote no for such a library...

I am also not convinced by the rationale behind the 
different-than-built-in-behaviour (sign-magnitude, overflow NaN...).

>"Chad Nelson" <chad.thecomfychair_at_[hidden]> wrote in message
>news:20110302202855.5cd8067f_at_ubuntu...
>
>>On Wed, 02 Mar 2011 18:18:33 +0100
>>Mathias Gaunard <mathias.gaunard_at_[hidden]> wrote:
>>
>> [...] Also, it seems that the object always contains the copy count
>> and a readonly boolean, which total 2 words, even if COW is disabled.
>
>Eliminating sixteen bits of overhead, on an integer that is almost
>always going to be larger than 64 bits (and often *much* larger), seems
>to be a premature optimization at best.

2 words are 64 bits on a 32 bit system and 128 bits on a 64 bit 
system...that's 25% of a 512 bit number.
In fact, for a fixed-sized integer with disabled COW, there are, AFAICT, two 
redundant std:size_ts and two bools (in addition to the data pointer and 
dynamic memory allocation) per integer...that certainly is not so easily 
dismissible, especially on a 64 bit system and especially if we also 
consider the code that maintains those 'useless' data members...Quite to the 
contrary, I would sooner accuse your approach of premature pessimization 
(have you even examined the assembly output of your code?).
In one post you complained that you are already tired of all the criticism 
on this list but please try to learn from this. The above misunderstanding 
and/or unwarranted dismissal of data layout/storage issues along with things 
like thinking that virtual inheritance is resolved at compile time, not 
knowing about the CRTP and misunderstanding of when to 'pass by value for 
performance' shows that you have holes in your knowledge which should make 
you more humble before honest criticism...
You also complained about not getting enough feedback in the last iteration 
of your library, IMHO your attitude (of fixing your library on your own set 
of use cases) again may have something to do with that. I remember way back 
then people were already criticising the hardwired dynamic allocation and 
data-layout&storage coupling design. I proposed that you extract algorithms 
into free functions that simply take and work on data ranges as this would 
both eliminate template bloat (in your current design the algorithms get 
replicated for every integer_t instantiation) and allow for cleaner 
implementation of different data storage strategies. As far as I remember 
you gave little heed to these concerns/suggestions or complained that that 
would be too much time consuming to implement while OTOH you spent who knows 
how much time on implementing COW which AFAICT turned to be a misguided 
premature optimization (of an a priori inefficient design)...
When I finally saw that you actually defended your hardwired dyn.mem. 
allocation by saying that it enables faster swaps I simply gave up on 
replying at that time deciding I'd wait for an official review (when 
boost.devel input has to be taken into consideration else you don't get 
accepted)...

ps. I'm sorry if some/all of the above mentioned issues were already 
resolved (I did not manage to read through the whole discussion before the 
review deadline)...

pps. I saw FFT being mentioned, you migh want to check out this 
http://drdobbs.com/cpp/199500857

ppps. You use a member-function-pointer based "unspecified bool type" 
implementation which produces suboptimal code on MSVC <10...IMO a plain 
operator bool would probably do just fine here as the built-in integral 
types also implicitly convert to bool...

-- 
"What Huxley teaches is that in the age of advanced technology, spiritual
devastation is more likely to come from an enemy with a smiling face than
from one whose countenance exudes suspicion and hate."
Neil Postman