Re: [boost] [Review] Shmem

From: David Abrahams (dave_at_[hidden])
Date: 2006-02-12 00:11:16

Ion Gaztañaga <igaztanaga_at_[hidden]> writes:

> Hi David,
>
> I think this Shmem review has become a general C++ usage/pattern
> flame war

I find that a rather self-fulfilling statement.  It's insulting to
characterize my expression of concern for this design principle as
flame.

> that does not benefit Boost or Shmem itself. 

I think if you tried to learn something from my comments, Shmem's
design would in fact benefit.

> So I will try to make some comments about your response. Due to my
> bad english, please don't see any irony, or provocation comment in
> my post. I don't know enough English to do that.
>
>>>> If you really *must* provide 2-phase construction, then please make it
>>>> a special mode that can be enabled with an #ifdef.
>>> I see you have strong opinions like me. So, no ifdefs, please. 
>> 
>> Then consider making the class with 2-phase initialization a different
>> type.
>
> What I want to know is if you consider a library rejection reason if 
> Shmem provides *both* RAII *and* open/close functions.

I consider a design that specifically accomodates a version of C++
with some of its features turned off at the expense of guarantees that
one can otherwise achieve -- especially if that expense is completely
avoidable -- cause for concern.  And based on your response to my
concerns so far I would be inclined to worry about the future of the
library and your responsiveness to other legitimate concerns.  All
that would tend to bias me towards voting against this library.  It's
not a reason for a "no" vote in and of itself, but if I had any energy
left to do an actual review, I would certainly be motivated to find
other areas where the design looked problematic to me.

>>> If you only have to use the constructor to open the shared memory I
>>> can't make boost::shmem::shared_memory a member of a class if the
>>> class has a different lifetime than the shared memory.
>> 
>> Use boost::optional or scoped_ptr.
>
> Isn't optional a two phase initialization? 

That's built into the condition you're trying to achieve: "if the
class has a different lifetime than the shared memory..."

> The optional may be or not constructed, and you have to check that
> before using optional. I'm trying to avoid dynamic allocation so
> scoped_ptr is not a good enough solution in my opinion.

The scoped pointer or optional would be employed by the *user* of
shmem who wants to achieve that difference in lifetime.

[BTW, I'm not sure that avoiding dynamic allocation is an appropriate
goal for Shmem -- not that it matters, since I'm not suggesting you
build dynamic allocation into your library.]

> I meant that with a class that wants to contain a pure-RAII object 
> without dynamic allocation/optional, it has to initialize the object in 
> the member constructor list. 

So?

> Imagine that depending constructor 
> arguments and the constructed other member, you want to open, 
> open_or_create or create a RAII object. Since you have to initialize 
> raii in the initializer list, Iyou can only use a constructor type. 

What is a "constructor type?"

> And for example, to open raii I need 3 arguments, and to create raii
> resource I need 4.
>
> class Holder
> {
>     RAII  raii;
>     public:
>     Holder(/*some conditions*/)
>        : raii(/*Create or open depending arguments,
>                 and other temporary results*/)
>     {}
> };

So make your constructors more flexible.

  class Holder
  {
      RAII  raii;
      public:
      Holder(/*some conditions*/)
       : raii( 
             generate_initializer( 
                 arguments and other temporary results 
             ) )
      {}
  };

If you use the parameter library it's especially easy to accomodate
this sort of interface.

>
>>> Using RAII only approach I have to
>>> dynamically allocate it. 
>> 
>> There's always optional.
>
> Like I've said, optional provides two-phase initialization, which is 
> something you want to avoid, no? 

Not when you are trying to achieve "different lifetimes."  That's
building two-phase into the problem statement!

> An empty optional is clearly "half-baked".

Only as much as a null pointer is.

>>> And to open another segment I can't reuse
>>> the object, I have to destroy it and allocate a new one.
>> 
>> There's no reason you can't make it reopenable.
>
> You are right. But I've understood that some boosters wanted a pure 
> constructor/destructor approach to open/close semantics, with no 
> reopen(). I may have understood it wrong, though.

I have no objection to reopen, FWIW.

>>> Default "empty" state (which is not the same as "zombie", because the 
>>> class is fully constructed) allows also move semantics between
>>> classes. 
>> 
>> Having such an empty "destructible-only" state is a an unfortunate but
>> necessary side effect of move optimization, but it is not necessary to
>> make such objects available for immediate use as the ctor does.  After
>> moving away, the object is impossible for the user to touch (unless
>> move() has been used explicitly).
>
> I agree. But I only wanted to point out that move semantics need an 
> empty state. I agree that it's clear that normally, once moved, you 
> clearly don't want to use it. 

Not only "don't want to" but normally, "can't."
That's my point.  The empty state does not need to be considered by
programmers of normal code.

> But consider a std::set with of 
> std::string objects that you have to fill from a a container of 
> c-strings (I think this problem was in Alexandrescu's last CUJ article). 
> If you want to avoid any overhead you can:
>
> std::string temp_string;
> auto it = source.begin(), itend = source.end();
>
> for( ;it != itend, ++it)
> {
>     temp_string = *it;
>     string_set.insert(std::move(temp_string));
> }

A moved-from object is not necessarily assignable.

> Clearly, the moved object reuse can produce optimal code with move 
> semantics. 

And just why is this more efficient than

    string_set.insert(std::string(*it))

??  Maybe it's not worth answering; I think this is really beside the point.

> Imagine now a container of shared_memory or file_mappings 
> (which are noncopyable but moveable). I think that a container of 
> shared_memory elements is more efficient than a container of 
> smart_ptr<shared_memory> and you don't need to pass through the pointer 
> syntax. 

??
Syntax has no runtime cost.

> So I think that the reuse of a moved object can produce optimal 
> code. And surely we will discover more uses to this "moved recycling" 
> concept.

Sounds like premature optimization to me.

>>> If you can only only the constructor and the destructor to open/close 
>>> the segment you can't move the object to the target, since the source 
>>> object can't be in a constructed state without owning the shared memory.
>>>
>>> I really don't like the "you should ONLY do this" approach. 
>> 
>> I don't know what you mean.
>
> When the first reviews commented the RAII absence, I immediately 
> proposed both RAII *and* open/close approach. The first one would 
> involve exceptions and the second one return values. But I've understood 
> that you were proposing *only* RAII approach, considering an additional 
> two-phase possibility approach a bad design. 

It usually is bad design, yes.  And in this case I don't see any
evidence that Shmem needs an exception to the rule more than any other
class.  All the arguments you've used would lead me to put two-phase
initialization interfaces in every class.

> So I've understood that you wanted to force me to use a RAII
> approach

I can't force you to do anything.

> under "we should keep the programmer away from this"
> excuse. I repeat: I *want* to provide RAII.  But not *only* RAII.

There's no excuse, and it's not about keeping the programmer away from
anything.  There's a good argument, and it has to do with being able
to reason about the code.

>>> That's because what now is cool, some years later is demonized. I
>>> want choice.  I don't like the approach of boost::thread, so I have
>>> to create a new boost::thread object every time I want to launch a
>>> thread. And I'm not the only one. With boost::thread, I can't have
>>> member boost::thread objects in classes (or I have to use optional<>
>>> like hacks)
>>>
>>> No silver bullet. No Good Thing (tm). If RAII is considered a good
>>> approach, I agree with you that I should provide it. But if fstream
>>> is good enough for the standard library, I think it can be a good
>>> model for Shmem without ifdefs.
>> 
>> That logic is flawed.  There are lots of examples of bad design in the
>> standard library.  fstream is hardly the worst.
>
> Yes. But at that time, surely they were designed under "good practices" 
> approach. Otherwise they wouldn't be in the standard. Only time will say 
> if "only RAII" approach won't be broken with future language features.

So let's throw all good design guidelines out the window, because
future unforseen language changes may make them obsolete.  Sorry, now
I *am* getting sarcastic.  This is starting to seem pointless.

>>> Apart from this, little discussion, now that you are in the Shmem review 
>>> thread, I would love if you could find time to review it. I think that 
>>> your C++ knowledge is very interesting for a library that tries to put 
>>> STL containers in shared memory and memory mapped files.
>> 
>> What I've been discussing here is one of the few deep aspects of my
>> C++ knowledge, and one of the few aspects that you're not likely to
>> get from others -- i.e. it is to some extent my unique contribution --
>> yet it seems like you're not really very interested in it.  That
>> doesn't leave me feeling very encouraged about further participation.
>
> I *really* understand your reasons, 

It doesn't sound like that, so far.

> and I *am* interested: that's why I *will* provide RAII (without
> wrappers, I repeat).

Then you clearly don't understand me at all.  I'm arguing that
interfaces that easily lead to zombie states make components harder to
use and code harder to think about.  Just providing a *way* to avoid
the zombie state does not help me know that I can operate on such an
object that I'm passed by reference without first making sure it's not
a zombie.

> But I want to have freedom to offer also a exception-less, two phase
> initialization so that the programmer has freedom to choose, without
> ifdefs, and ready to "move recycling". 

There ways to get that without 2-phase construction.

> If RAII is so good, the programmer will use it. But I want people
> with no exception support (which is common is restricted
> environments) or exception-alergic to have an alternative.

Then allow them to check for successful construction after the fact,
or if you *really* must, provide a different type that has the 2-phase
feature.

> I think that apart from this C++ aspect of your knowledge, there is
> plenty of your C++ knowledge that you can use in other aspects of
> the library. And I'm really interested in them. Specially, if they
> are related to correct uses or better idioms like the previous
> one. And I want to support those idioms. I just want to provide some
> alternatives apart from your advices.

I think I've used up most of my available energy on this one.  :(

-- 
Dave Abrahams
Boost Consulting
www.boost-consulting.com