Boost logo

Boost :

Subject: Re: [boost] [pimpl] Some comments
From: Vladimir Batov (Vladimir.Batov_at_[hidden])
Date: 2014-06-05 21:51:18


Andrzej, Thank you for your quick reply. :-) ... I did not even know
Glen already advertised the upcoming review. Please find my replies
below. (I snipped some for brevity. My apologies)

V.

On 06/06/2014 07:21 AM, Andrzej Krzemienski wrote:
> ...
>
> [1] The main point in using pimpl idiom (at least my) is to avoid header
> inclusion, which may grow compilation time too much (at the expense of
> certain run-time cost). But in order to use Boost.Pimpl I do need to
> include a header file, which in turn includes more header files. This is
> arguably less headers that in normal non-pimpl implementation, but it is
> more than if I used a raw pointer. And this is what I finally did in my
> work: switched to raw pointers for implementing piml in order to get the
> build perform faster. (I didn't measure the improvement though.)

Well, I feel you mix up two things here: pre-build analysis and the
build as such. The number of included headers has impact on the first
phase -- pre-build analysis -- as 'make' has to check those
included-files timestamps. I do not believe it's such an issue as I do
not expect to notice much/any performance differences from checking 1 of
10 timestamps. So, if you have pimpl.hpp included with a few other Boost
headers thrown into the mix, then 'yes' it does increase the time of
pre-build analysis... which is negligible compared with the time of the
actual build. Given those mentioned Boost headers do not change, they do
not trigger re-build. That said, when re-build is triggered by other
code, those headers to need to be compiled and that takes time.

Now we are getting to the build itself. The thing is that build performs
the fastest when it, well, does not build. And that's what Pimpl allows
you to achieve. So, in short, if you deploy the Pimpl idiom, then you
manage to avoid re-builds. The time/performance difference between your
solution and mine will only be in pre-build-analysis time... which I
claim to be fairly minimal if noticeable at all.

As for doing Pimpl yourself vs. a library solution, then it's a
different matter. I'll take the latter any day... and will make sure
people on my project do the same... if they want their code to pass
through review. :-)

> [2] Boost.Pimpl is good for everyone: people who like value semantics get
> the value semantic interface, and people who like shated_ptr's get
> shared_ptr semantics. Call me selfish or biased, but I believe using
> shared_ptr's should be banned (at least in 90% of the cases where it is
> currently being used), and the fact that you advertise it first in the docs
> makes me uneasy.

We might be developing for different industries. I've been working for
rail and air for the last 15 years and there shared data are everywhere.
Rail network topology is only one, aircraft data, airline schedules are
all shared by many threads. So, my focus on shared_ptr-based
pimpl::pointer_semantics is clearly biased.

> It looks from the docs that 'pointer_semantics' implements something like a
> flyweight.

Yes, it is a variation of the Flyweight idiom... as shared_ptr and raw
pointers are... Although I personally do not like the name as I find it
confusing. In GoF it is the shared object (big and expensive to copy)
which is called "flyweight object". To it is the proxies (shared_ptr,
raw pointers) which look like "flyweight". :-)

> This is so against value semantics. One of the expectations of
> types returned by value is that changing the original does no affect the
> copy:
>
> T a = c;
> T b = c;
> zap(a);
> assert(b==c && a!=b);
>
> Although the author of class Book knows that it is implemented with
> pointer_semantics, the users of the class are likely not to know it, and
> since Book doesn't look like a pointer, they will be expecting value
> semantics, and will be severely punished.

Well, I certainly agree that value-semantics classes are quite different
from pointer-semantics classes... and it is expected that user knows
what he/she is using. Pimpl tries to help by making it clear in the
declaration:

class Book : public pimpl<Book>::pointer_semantics
class Book : public pimpl<Book>::value_semantics

To me "doesn't look like a pointer" sounds like a weak argument because
there are many things in C++ which do not look like a pointer:

1) non-const reference;
2) typedef shared_ptr<foo> my_foo;
3) many classes in X Window System (like Widget) are actually pointers.

What I am driving at is that the user ultimately needs to know what
class and its behavior actually are to use it effectively.

That said, I do acknowledge a potential for mis-use. In my code I
address it with "const". That is, say, shared_ptr<foo const> is passed
around and has to be copied to shared_ptr<foo> before being modified.

> [3] The docs advertize the definition of operator== as a member function.
> This is against a typical recommendation that it should be a free function
> in order to avoid asymmetrical behavior in certain cases. Does your library
> allow the definition of a non-member operator==? If so, I would encourage
> you to reflect that in the docs.

op==() as a member works well for object of he same type, like pimpl1 ==
pimpl2... and for pimpls I believe it's all what's needed. I might be
wrong though (just have not had practical applications of different
sort). I'll look into it though.

> [4] This page:
> http://yet-another-user.github.io/boost.pimpl/the_boost_pimpl_library/pimpl__null___and_strongly_exception_safe_constructors.html
> says "The advantage of the 'built-in' *Book::null()* [...] is that it makes
> possible an implementation of strongly exception-safe constructors, the
> copy constructor" There seems to be something wrong with this statement.
> How can a strong (commit-or-rollback) guarantee apply to constructors?

I could not remember what I based those statements on. :-( I started
researching and writing a reply but got totally tangled up with all
those exception-guarantee complexities... Got totally confused...
removed the part.

> ...
> While I find the value_semantics interface useful, I am strongly against
> even offering the pointer_semantics interface.

Hmm, that "strongly against" indicates some conviction and I think Gavin
echoed that view in a later post. I'll probably need to revisit my lib
designs as I am using it a lot. For example, right now I read a DB,
cache it. Then everyone has read-only access to records via pimpl-like
shared_ptr-based facility. It's all kosher as read/access requests
mutexed and fast. Where is the problem? I seem to be missing something big.


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk