Hi, I don't know if this is the correct ML to talk about this subject, but as there was recently a discussion about wether Boost should or not use Pimpl, I have decided to post this here. In http://www.gotw.ca/publications/mill05.htm H. Sutter states "There are four main alternative disciplines: 1. Put all private data (but not functions) into XImpl. 2. Put all private members into XImpl. 3. Put all private and protected members into XImpl. 4. Make XImpl entirely the class that X would have been, and write X as only the public interface made up entirely of simple forwarding functions (another handle/body variant)." My experience is that I add more private member functions than private member data. I have explored a different variant 5. Put all private funtion and static data (but not instance data) into XImpl. XImpl needs only to store the back pointer. While this approach doesn't encapsulates all the private members has some advantages: * The XImpl class has no inherent instance data, so no space overhead * The class doesn't needs to store any XImpl pointer as there is no data to maintain, so no need to allocate/deallocate it. * Reduce the performance overhead of the Pimpl idiom. The drawbacks are: * Need to include the headers needed for the private instance data. * There is yet a minimal performance overhead on the construction of the temporary XImpl class and the derreference of the back pointer. Next follows an implementation exaple. * Grant friend access to the private implementation class class C { public: void f(); void g(); protected: // ... private: struct impl; friend struct impl; // the private part contains only instance data and virtual functions int priv; }; * Identify the private functions and the private static data and put them on the implementation class struct C::impl { C::impl(C* thisC) : that(*thisC) {} void ifct() { that.priv = ... // use of private data that.g(); // call to a public function jfct(); // call another private function } C& that; static int icount; }; * Prefix all the ex private function calls by impl(this) impl(this).ifct(); * Access without constraints the impl static data impl::icount; There is a proposal from Francis Glassborow, N1742: Auxiliary class interfaces:<http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2004/n1742.pdf> that address this problem in a general context. What do you think of this variant? Best, _____________________ Vicente Juan Botet Escribá
On Thu, Apr 1, 2010 at 4:13 PM, vicente.botet <vicente.botet@wanadoo.fr> wrote:
Hi,
I don't know if this is the correct ML to talk about this subject, but as there was recently a discussion about wether Boost should or not use Pimpl, I have decided to post this here.
In http://www.gotw.ca/publications/mill05.htm H. Sutter
My experience is that I add more private member functions than private member data. I have explored a different variant 5. Put all private funtion and static data (but not instance data) into XImpl. XImpl needs only to store the back pointer.
While this approach doesn't encapsulates all the private members has some advantages: * The XImpl class has no inherent instance data, so no space overhead * The class doesn't needs to store any XImpl pointer as there is no data to maintain, so no need to allocate/deallocate it. * Reduce the performance overhead of the Pimpl idiom.
The drawbacks are: * Need to include the headers needed for the private instance data. * There is yet a minimal performance overhead on the construction of the temporary XImpl class and the derreference of the back pointer.
I must be missing something. In '5' what is left of Pimpl that is worthwhile? ie I think I understand the usual benefits of Pimpl - data hiding, changing instance data without changing the h file and forcing client compiles, etc. Aren't all these benefits lost? What's left. Alternatively, how is 5 better than a normal C++ class with no Pimpl? Is this an April Fool's thing? :-) By the way, although Herb Sutter warns against it in that same article, I've experimented with a 'Reckless' Pimpl that stores the data (typelessly) in the class: class C { struct impl; // forward declare RecklessPimpl<impl, 24> pimpl; }; so RecklessPimpl becomes the storage for struct impl. Since the size is not known, you supply it (eg 24). Now, there are obvious dangers to this, and Herb's article lists them. Surprisingly, I think I've managed to work around them all. ie - once in the cpp implementation file for class C, you can check that the size is OK: static_assert(sizeof(impl) == sizeof(Pimpl)); // or can use <= - alignment can also be assured - the constructor for RecklessPimpl can properly construct impl with inplace new - etc All this can be automatic in the template of RecklessPimpl, thus it isn't really so reckless after all. The resulting Pimpl is like a smart pointer in that calls to the impl look like pointer calls: pimpl->function(); etc. Of course if sizeof(impl) is increased too much, the h file needs to change and clients need to recompile, but it still minimizes the effects of changes, and hides private implementation details. My main use was to avoid things like windows.h, which tends to screw up code if it is only included _sometimes_ (ie it is more of an all or nothing thing!). Like so: struct CRITICAL_SECTION; // forward declared instead of #include <windows.h> RecklessPimpl<CRITICAL_SECTION, 32> csPimpl; Which worked well because the sizeof(CRITICAL_SECTION) didn't change. If anyone was really interested, I could probably submit it to boost when I find a bit more time (post-BoostCon). Tony
Gottlob Frege wrote:
On Thu, Apr 1, 2010 at 4:13 PM, vicente.botet <vicente.botet@wanadoo.fr> wrote:
Hi,
I don't know if this is the correct ML to talk about this subject, but as there was recently a discussion about wether Boost should or not use Pimpl, I have decided to post this here.
In http://www.gotw.ca/publications/mill05.htm H. Sutter
My experience is that I add more private member functions than private member data. I have explored a different variant 5. Put all private funtion and static data (but not instance data) into XImpl. XImpl needs only to store the back pointer.
While this approach doesn't encapsulates all the private members has some advantages: * The XImpl class has no inherent instance data, so no space overhead * The class doesn't needs to store any XImpl pointer as there is no data to maintain, so no need to allocate/deallocate it. * Reduce the performance overhead of the Pimpl idiom.
The drawbacks are: * Need to include the headers needed for the private instance data. * There is yet a minimal performance overhead on the construction of the temporary XImpl class and the derreference of the back pointer.
I must be missing something. In '5' what is left of Pimpl that is worthwhile? ie I think I understand the usual benefits of Pimpl - data hiding, changing instance data without changing the h file and forcing client compiles, etc. Aren't all these benefits lost? What's left. Alternatively, how is 5 better than a normal C++ class with no Pimpl? Is this an April Fool's thing? :-)
No, this is not a April Fool. :( 5 can not be considered really a Pimpl as there is not pointer as such. As I said even if no instance data hiding is provided by this variant it provides function hiding. I have used it to implement the Methods for state pattern on which there are a lot of internal function for each transition between states. A change in the implementation of the FSM means adding removing transition which means modifying the class interface. The variant I purpose avoid changing the header file in this case. Gottlob Frege wrote:
By the way, although Herb Sutter warns against it in that same article, I've experimented with a 'Reckless' Pimpl that stores the data (typelessly) in the class:
class C { struct impl; // forward declare RecklessPimpl<impl, 24> pimpl; };
so RecklessPimpl becomes the storage for struct impl. Since the size is not known, you supply it (eg 24).
Now, there are obvious dangers to this, and Herb's article lists them. Surprisingly, I think I've managed to work around them all.
ie - once in the cpp implementation file for class C, you can check that the size is OK: static_assert(sizeof(impl) == sizeof(Pimpl)); // or can use <= - alignment can also be assured
How? Does you RecklessPimpl class have an alignment parameter? Gottlob Frege wrote:
- the constructor for RecklessPimpl can properly construct impl with inplace new - etc
All this can be automatic in the template of RecklessPimpl, thus it isn't really so reckless after all.
The resulting Pimpl is like a smart pointer in that calls to the impl look like pointer calls: pimpl->function(); etc.
Of course if sizeof(impl) is increased too much, the h file needs to change and clients need to recompile, but it still minimizes the effects of changes, and hides private implementation details.
My main use was to avoid things like windows.h, which tends to screw up code if it is only included _sometimes_ (ie it is more of an all or nothing thing!). Like so:
struct CRITICAL_SECTION; // forward declared instead of #include <windows.h>
RecklessPimpl<CRITICAL_SECTION, 32> csPimpl;
Which worked well because the sizeof(CRITICAL_SECTION) didn't change.
If anyone was really interested, I could probably submit it to boost when I find a bit more time (post-BoostCon). Tony
Waiting for a proper Boost submission maybe you can share with us, which of the warnings H. Sutter signal against RecklessPimpl do you solve with your implementation (alignment been the most important issue to me). Could you share also your implementation. Best, Vicente -- View this message in context: http://old.nabble.com/A-Pimpl-variant-tp28113118p28116567.html Sent from the Boost - Dev mailing list archive at Nabble.com.
----- Original Message ----- From: "Vicente Botet Escriba" <vicente.botet@wanadoo.fr> ... Waiting for a proper Boost submission ...
There has been one Pimpl submission sitting in the queue for over two years (see http://www.boost.org/community/review_schedule.html). I am not sure if that corresponds to your definition of "proper" though. ;-) Best, Vladimir.
Vladimir Batov-4 wrote:
----- Original Message ----- From: "Vicente Botet Escriba" <vicente.botet@wanadoo.fr> ... Waiting for a proper Boost submission ...
There has been one Pimpl submission sitting in the queue for over two years (see http://www.boost.org/community/review_schedule.html). I am not sure if that corresponds to your definition of "proper" though. ;-)
Hi, I'm aware of your proposal Vladimir :) I was requesting more information respect the proposal of Tony before I could be able to submit a "proper" submission. "If anyone was really interested, I could probably submit it to boost when I find a bit more time (post-BoostCon). " BTW, does your library provides a RecklessPimpl implementation or something like that? Best, Vicente -- View this message in context: http://old.nabble.com/A-Pimpl-variant-tp28113118p28117150.html Sent from the Boost - Dev mailing list archive at Nabble.com.
----- Original Message ----- From: "Vicente Botet Escriba" <vicente.botet@wanadoo.fr> ... Hi, I'm aware of your proposal Vladimir :) ... BTW, does your library provides a RecklessPimpl implementation or something like that?
You are right, it does not. For quite some time I shared H. Sutter's point of view that the RecklessPimpl-based solution was, well, evil. :-) Then, I read Tony's explanations and that did not look that scary to me after all... if as Tony's suggested all the "nastiness" is encapsulated in RecklessPimpl. Now that I am thinking of the RecklessPimpl-based Pimpl, I see that it does provide the implementation-hiding property. However, what troubles me is that we seem to lose the other important Pimpl property -- the separation of the interface and implementation. Namely, if the internal implementation size changes, it has to be reflected in the public header. So, all the user-code has to be recompiled. Still, elimination of the memory-allocation-related overhead of the RecklessPimpl-based Pimpl looked very attractive. Then, I thought that a fixed-size allocator for the XImpl implementation class might be a good-enough main-stream alternative. Granted that a fixed-size allocator might not be as fast as allocation on the stack. More so, if my experience is of any guidance, I overwhelmingly use pimpls with pointer semantics. In my code they outweigh pimps with value semantics like 100:1. I am under impression that the RecklessPimpl-based Pimpl only provides value semantics. All in all, IMHO the RecklessPimpl-based Pimpl might be a nice fit for certain tasks. However, I am under impression that it is not generic enough... and, in fact, not Pimpl enough as it loses the separation of the interface and implementation and does not provide pointer semantics. All IMHO, of course. Best, V.
On Fri, Apr 2, 2010 at 8:13 PM, Vladimir Batov <vbatov@people.net.au> wrote:
Now that I am thinking of the RecklessPimpl-based Pimpl, I see that it does provide the implementation-hiding property. However, what troubles me is that we seem to lose the other important Pimpl property -- the separation of the interface and implementation. Namely, if the internal implementation size changes, it has to be reflected in the public header. So, all the user-code has to be recompiled.
Yes, that is the limitation. As in my example, when I first wrote it, I was hiding objects whose size wasn't about to change (eg CRITICAL_SECTION). There was also an option to over-allocate 'extra room' by using a size-estimate that was too large. With another template flag, you could say whether you wanted the size check to be exact or 'big enough'. Either way, it doesn't completely eliminate client-recompiles, but it can lessen them. Including for those cases, like your #5 where you are adding private functions, etc.
More so, if my experience is of any guidance, I overwhelmingly use pimpls with pointer semantics. In my code they outweigh pimps with value semantics like 100:1. I am under impression that the RecklessPimpl-based Pimpl only provides value semantics.
All in all, IMHO the RecklessPimpl-based Pimpl might be a nice fit for certain tasks. However, I am under impression that it is not generic enough... and, in fact, not Pimpl enough as it loses the separation of the interface and implementation and does not provide pointer semantics. All IMHO, of course.
some of us value value sematics. :-) RecklessPimpl obviously isn't for all occasions, but it might make a nice complement in a package of various Pimpl options. Tony
How? Does you RecklessPimpl class have an alignment parameter?
it would have a default alignment parameter. Defaulting to max alignment based on the size parameter. Ie a 4 byte size would default to 4 byte alignment. Size >= 8 means 8 byte alignment (typically). The actual template to find a primitive type with the right alignment based on size is pretty hairy. Not sure if I ever had a version that I was happy with.
Waiting for a proper Boost submission maybe you can share with us, which of the warnings H. Sutter signal against RecklessPimpl do you solve with your implementation (alignment been the most important issue to me).
I think all of them. :-)
Could you share also your implementation.
I would need to reimplement it from scratch as I don't have it anymore. But I did it once so I'm sure I could do it again. It was actually more general than pimpl. More of a manager of in place construction and storage, with policies. Possibly overlapping with some of the internals of boost.optional. It also suffered from the forwarding problem when passing parameters through to the underlying constructor. Not much you can do to fix that, yet. Tony
AMDG Gottlob Frege wrote:
How? Does you RecklessPimpl class have an alignment parameter?
it would have a default alignment parameter. Defaulting to max alignment based on the size parameter. Ie a 4 byte size would default to 4 byte alignment. Size >= 8 means 8 byte alignment (typically). The actual template to find a primitive type with the right alignment based on size is pretty hairy. Not sure if I ever had a version that I was happy with.
Can't you just use boost::aligned_storage? In Christ, Steven Watanabe
On Fri, Apr 2, 2010 at 11:14 AM, Steven Watanabe <watanabesj@gmail.com> wrote:
Can't you just use boost::aligned_storage?
In Christ, Steven Watanabe
That would definitely help (can't recall if it was around back then). Either way, it doesn't solve the other problem of "I have a 12 byte object, what is its worse-case alignment?" I would guess 8, given today's machines, and align it to a double. Actually, no. I always start thinking that way, then realize I'm wrong. The alignment would be 4 (for today's typical machine). ie the largest sizeof(primitive_type) such that it divides into the size. And now that I think about it, maybe there is no reason to not use alignment of 12. Same diff I suppose. So maybe aligned_storage<12, 12> is all I need. I now I can't remember if the original hard problem was getting an answer of 8 given a size of 12 (which is wrong anyhow) or whether I'm forgetting some detail that bit me later. So a question. Given: struct { int a, b, c; }; is there any difference saying it has an alignment of 12 vs an alignment of 4? (assuming sizeof(int) == 4)? And/or is there any difference between boost::aligned_storage<12, 4> and boost_aligned_storage<12, 12>? Tony
vicente.botet wrote:
My experience is that I add more private member functions than private member data. I have explored a different variant 5. Put all private funtion and static data (but not instance data) into XImpl. XImpl needs only to store the back pointer.
[snip]
class C { public: void f(); void g(); protected: // ... private: struct impl; friend struct impl; // the private part contains only instance data and virtual functions int priv; };
* Identify the private functions and the private static data and put them on the implementation class
struct C::impl { C::impl(C* thisC) : that(*thisC) {} void ifct() { that.priv = ... // use of private data that.g(); // call to a public function jfct(); // call another private function } C& that; static int icount; };
* Prefix all the ex private function calls by impl(this) impl(this).ifct(); * Access without constraints the impl static data impl::icount;
Wouldn't it be simpler to use static member functions? struct C::impl { static void ifct(C * _this) { _this->priv = ... // use private data _this->g(); // call public member function ... } }; Using it from C member functions would be straightforward: void C::f() { impl::ifct(this); // compare: impl(this).ifct(); ++impl::icount; } The definition of impl is simpler in my version and invoking impl member functions is one (!) character shorter. Your version requires an implicit this argument from which "that" is found; mine makes your "that" the sole argument, so its potentially faster. For those that like "this->" in front of member access, mine even looks similar (in impl's member functions). _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
----- Original Message ----- From: "Stewart, Robert" <Robert.Stewart@sig.com> To: <boost@lists.boost.org> Sent: Friday, April 02, 2010 7:00 PM Subject: Re: [boost] A Pimpl variant
vicente.botet wrote:
My experience is that I add more private member functions than private member data. I have explored a different variant 5. Put all private funtion and static data (but not instance data) into XImpl. XImpl needs only to store the back pointer.
[snip]
Wouldn't it be simpler to use static member functions?
struct C::impl { static void ifct(C * _this) { _this->priv = ... // use private data _this->g(); // call public member function ... } };
Using it from C member functions would be straightforward:
void C::f() { impl::ifct(this); // compare: impl(this).ifct(); ++impl::icount; }
The definition of impl is simpler in my version and invoking impl member functions is one (!) character shorter. Your version requires an implicit this argument from which "that" is found; mine makes your "that" the sole argument, so its potentially faster. For those that like "this->" in front of member access, mine even looks similar (in impl's member functions).
Hi Robert, I have considered also this possibility. One of the advantage of using an instance implementation is that we can inherit from the implementation, while using static functions doesn't allows it. Of course if a class should be able to inherit from the impl class, the class declaration must be moved to a header file. Otherwise, you approach could be shorter, simpler and even more efficient. Note that the use of that. or this_-> is just a matter of taste and it is not realy the matter of this post. Respect to the comaparation between impl::ifct(this); and impl(this).ifct(); I want to add that the class C could declare a inline function This() that return impl(this) and be used as follows This().ifct(); which is 3 (!!!) characters shorter. But I don't think that we can compare solutions taking in account how many characters the solution use. There is another advantage to use a This() function. We can move to a complete PImpl implementation with minor changes in the code. Best, Vicente
participants (6)
-
Gottlob Frege -
Steven Watanabe -
Stewart, Robert -
Vicente Botet Escriba -
vicente.botet
-
Vladimir Batov