Re: [boost] [function] function wrapping withnoexceptionsafetyguarantee

Subject: Re: [boost] [function] function wrapping withnoexceptionsafetyguarantee
From: Domagoj Saric (dsaritz_at_[hidden])
Date: 2010-11-08 19:22:11

"Daniel Walker" <daniel.j.walker_at_[hidden]> wrote in message 
news:AANLkTim_OqV7nV_Q7R6YanUHFMYPkM8i3Wsm6FpGrRNX_at_mail.gmail.com...
> On Sat, Oct 30, 2010 at 1:24 PM, Domagoj Saric <dsaritz_at_[hidden]> wrote:
>>
>> "Daniel Walker" <daniel.j.walker_at_[hidden]> wrote in message
>> news:AANLkTimSdhEy=bQxj=mKkJG0d48kuCpXYnQz9DK2O_A0_at_mail.gmail.com...
>>
>>> So, I think the current boost::function implementation is certainly
>>> the right default, since many users would not appreciate doubling the
>>> static space overhead for a time savings of less than 10% per call.
>>
>> I already explained to you that there is no 'double space overhead' with 
>> the
>> condition that the change is done right or the linker is able to merge
>> identical objects, neither of which is true in the case you posted.
>
> I'll try to give a brief outline to illustrate why adding a static
> vtable means there are at least two static vtables per signature.

Even if there were at least two vtables (which there aren't as already 
explained) this still does not constitute a 'double space overhead' which is 
what you were claiming...

> Here
> is what boost::function must do when you assign a target:
>
> static target_type stored_vtable = // target function vtable
> if(assign(stored_vtable, target_function)) // clone the function
>    vtable = &stored_vtable;
> else
>    vtable = NULL;

No, this is what current boost::function does not what it 'must' do...(I'm 
sorry I have to be picky because this whole thread has issues with lack of 
precise terminology, shifting/redefining the basic terms of the discussion 
and reasserting already proven-false claims)...

> Now, if instead we want vtable to point to a static empty object, we
> need to do something like the following.
>
> static target_type stored_vtable = // target function vtable
> if(assign(stored_vtable, target_function)) // clone the function
>    vtable = &stored_vtable;
> else {
>   static empty_target_type stored_empty_vtable = // empty target vtable
>   assign(stored_empty_vtable, empty_function) // does not fail
>   vtable = stored_empty_vtable;
> }

No, this is what your implementation does (which is why it is 'wrong', as 
already explained) not what 'we need to do'...

> Again this is a rough outline of boost::function assignment, but it
> gives you the idea.

Thanks, I have a 'rough' idea of boost::function assignment as I've pretty 
much reimplemented it...
If you want you can take a look at the assembly listings presented here 
http://lists.boost.org/Archives/boost/2010/10/172593.php that also 
demonstrate assignment...

> The reason you need two static initializations is
> that target_type and empty_target_type may not be the same type.

Right, except that saying that they _may not_ be is also implying that they 
_may be_... which basically means that your claim to a 'double space 
overhead' is based on a 'maybe' something that even when happens still does 
not imply/result in a 'double space overhead'...

>> The 'double' overhead you got is actually not 'double' but one 
>> unnecessary
>> vtable extra:
>> - even in your implementation, if you assigned more (different typed)
>> targets to a boost::function you'd get an additional vtable for each of
>> those (different typed) targets...so the overhead, as said, is not double
>> but one extra...
>
> In the worst case scenario one extra vtable per signature _is_ double;
> i.e. in the worst case, the largest increase in overhead you can
> possibly see is 100%. I do not mean that there are at most two
> vtables; I mean there are at least two vtables instead of one.

Your original claim was just 'double' not 'double in the worst case'...
Furthermore, your 'worst' case (where you only store one type) is actually a 
'no use case'/antipattern as you yourself actually claim in this post 
http://lists.boost.org/Archives/boost/2010/11/172798.php concluding to the 
effect that this case is not something worth considering...

Finally, for the umpteenth time, there are not at least two vtables instead 
of one:

>> - the problem with your implementation is that you copied the original
>> assign() member function for your new "empty target" function copying 
>> with
>> it the local static vtable...to do it right the vtable must not be a 
>> local
>> (template) function static...
>
> It doesn't matter whether the empty static assignment is in a
> different member function or not. boost::function will still require
> two static assignments: one for the actual target and one for the
> empty target.

No we do not, as I already explained in more detail and as pointed out by 
Peter Dimov (and maybe others)...will you please stop reasserting this false 
claim?

This is what your 'wrong' implementation does/causes because it has two 
member functions with local static vtables...In the original implementation, 
and somewhat similarly in mine, there is one member function template with a 
local static vtable that depends (through the template parameters) on the 
type/signature of the boost::function<> and on the type of the assigned 
target...from this alone it directly follows that for targets of same type 
assigned to objects of one boost::function<> instantiation only a single 
vtable will be used...or IOW if the type of the empty target coincides with 
the type of any other target assigned to an object of the same instantiation 
_no_ additional vtable will be generated...
QeD

If you still don't believe fix your implementation and/or examine the binary 
generated for Peter Dimov's example and see for yourself...

>> Not to mention, as also already explained in more detail (and pointed to 
>> by
>> Nevin), this 'overhead' of a few static pointers is completely 
>> insignificant
>> compared to various related code bloat issues...
>
> On some platforms the overhead of a few static pointers matters a
> great deal. Regardless, there's no reason to increase the static space
> overhead at all, if it does not improve time performance
> commensurately.

Right, except that arguing about the overhead of a few (const) static 
pointers w/o also mentioning the reduction in total time _and_ space 
overhead that this much smaller overhead enables is a straw man argument...
Just as the implication that there is no time performance gain is simply 
false (as shown in the last post)...

For example, the fact that the vtable in my implementation is ~3 times 
larger than the original matters hardly anything if it helps to save the 
equivalent of say 50 vtables in code size on each assignment (as 
demonstrated in the post linked to above or also here 
http://lists.boost.org/Archives/boost/2010/11/172682.php)...

"if ( empty() ) push-something-on-the-stack" can generate larger code 
(replicated on every place it is inlined) than the size of one vtable...not 
to mention the real if ( empty() ) do-all-of-the-boost-exception-magic 
code...

Even a call to the boost::function<> safe_bool operator on MSVC 9 or earlier 
will probably generate larger code than a vtable...

...

-- 
"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