Re: [boost] [xint] Boost.XInt formal review

Subject: Re: [boost] [xint] Boost.XInt formal review
From: Chad Nelson (chad.thecomfychair_at_[hidden])
Date: 2011-03-05 20:55:37

On Sat, 05 Mar 2011 13:41:16 -0800
"Jeffrey Lee Hellrung, Jr." <jhellrung_at_[hidden]> wrote:

> Regarding the feedback from previous versions, you should understand 
> that two major (to me, at least) suggestions from previous versions
> were:
>
> - Eliminate COW entirely, or, at the very least, don't make it 
> mandatory. [...] Yes, this is largely an implementation concern, but
> I think a valid concern nonetheless.

The performance numbers indicated that it provided a significant
improvement. Those numbers will presumably change when I've implemented
the improvements that have been suggested recently, but in the unlikely
case that they still show it to be a significant improvement, I'd be
very reluctant to remove it.

> - Separate the arithmetic algorithms from the data structures,
> defining a BigInt concept or group of concepts in the process.
> Regarding the former, it appears we still don't have a definitive 
> benchmark comparing COW with move emulation, and one would be nice,
> but we can still predict how many deep copies each strategy will
> require for a given expression, and except in constrained situations
> where copies are never modified, I don't think COW gives any benefit.
> Sometimes COW is worse, since you can't explicitly transfer
> ownership.  Feel free to prove me wrong. [...]

I'll be sure to report the results, either way.

> Regarding the lack of feedback on your *seventh* iteration, it's 
> unfortunate, but I think, throughout your development process, that
> it was reasonable to expect:
> - that not everyone is going to inspect every iteration of your
> library; and
> - that you will get (by far?) the most of amount of feedback
> (including critical, almost negative, feedback) on the version you 
> submit for inclusion in boost.

I naively assumed that those who showed such passion against the
earlier iterations would at least look at the later one to see if their
pet peeve had been addressed.

> For what it's worth, I will vote to not accept, but I will try to 
> summarize my reasons for doing so in a separate email.

In any case, thank you for your comments. I'm a little puzzled why you
would vote to reject it though, instead of a conditional acceptance.
You seem to have clear conditions you want resolved.

>> I have no formal training on big-O notation, only what knowledge I've
>> managed to pick up over the years of trying to interpret discussions
>> of programming algorithms. If half of roughly n^2 is somehow
>> considered equal to n^2 in big-O notation, then obviously that isn't
>> sufficient.
>
> I think a quick trip to wikipedia, as suggested by Robert, should 
> convince you. 

That was one of the sources of my informal knowledge on the subject, and
at least the last time I looked, it explained nothing that I could see
about why O(n/2) == O(n).

>>> Can you give an example of the internals where you find
>>> copy-on-write necessary?
>>
>> Only the benchmarks that I came up with before, and referred you to
>> earlier in this thread. They may well change when I correct the
>> pass-by-value stuff.
>
> Sorry if this wasn't clear, so let me rephrase.  Which implementation 
> function or block of code necessitates using COW to eliminate 
> unnecessary copying?  This doesn't require benchmarking, only code
> analysis.

I don't have that information at present. I'll certainly let you know
if I discover specific places.

>>>> I had a publicly-available make_unique function in an earlier
>>>> iteration of the library, and was criticized for it, so I removed
>>>> it. I don't remember if it was you personally doing the
>>>> criticizing, but you-the-Boost-community can't have it both
>>>> ways. :-)
>>>
>>> Well this tagged copy constructor pretty much amounts to the same
>>> thing, doesn't it?  Only less efficient, since it will actually
>>> perform a deep copy in situations where it's not needed, e.g., when
>>> the object is already in the thread you want it to be owned in.
>>
>> Only if the person is foolish enough to explicitly tell the library
>> to do so. I'd like to think that any programmer using the library
>> wouldn't be that foolish.
>
> You didn't really address my primary comment.  You can put lipstick on
> a pig, but it's still a pig.  You've just renamed make_unique into a 
> constructor.  Is that a fair analysis?

Yes. The complaint, as I recall, wasn't that the make_unique function
existed, but that it was part of the interface and meant to be used or
at least usable by client code. I didn't understand it either.

>>> And more cryptic in client code.  By the way, is there ever any
>>> reason to set the boolean force_thread_safety to false?
>>
>> It should essentially *always* be false, that's why false is the
>> default value. [...]
>
> Sorry, is there any reason to set the boolean force_thread_safety 
> *explicitly* to false?  I suspect the answer is "no", so I would 
> consider this an inferior design [...]

Duplicating the code of that constructor function, with what I think
would be a single-line change, would be a superior design? I'm not
trying to be a pain, I'm honestly baffled at that assertion.
-- 
Chad Nelson
Oak Circle Software, Inc.
*
*
*