Ion Gaztañaga <igaztanaga@gmail.com> 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