pimpl optimization

[mliptak]

Hi group,
suppose having this code using PIMPL:

// a.hh
class A
{
struct Impl;
public:
A();

int Member1() const
{ return mrMember; }
int Member2() const;
private:
Impl* mpImpl;
int& mrMember;
};

// a.cc
struct A::Impl
{
int mMember;
};

A()
: mpImpl(new Impl)
, mrMember(mpImpl->mMember)
{}

int A::Member2() const
{ return mpImpl->mMember; }
// end of code

The member functions Member1() and Member2() should return the same
value at all times, except for the Member1() operation can be inlined
while Member2() cannot.
Does it make sense for certain members, which are hidden in sturct
Impl, to expose references in the header file so the accessor
operations can be inlined?
Is it better, from the design point of view, to have the member
attribute in class A and have its reference in struct Impl so Impl
functions can access it? (I am assuming from performance point of
view it should be the same as the code above).
Thanks
m.

 

[Dave Rahardja]

Hi group,
suppose having this code using PIMPL:

// a.hh
class A
{
struct Impl;
public:
A();

int Member1() const
{ return mrMember; }
int Member2() const;
private:
Impl* mpImpl;
int& mrMember;
};

// a.cc
struct A::Impl
{
int mMember;
};

A()
: mpImpl(new Impl)
, mrMember(mpImpl->mMember)
{}

int A::Member2() const
{ return mpImpl->mMember; }
// end of code

The member functions Member1() and Member2() should return the same
value at all times, except for the Member1() operation can be inlined
while Member2() cannot.
Does it make sense for certain members, which are hidden in sturct
Impl, to expose references in the header file so the accessor
operations can be inlined?
Is it better, from the design point of view, to have the member
attribute in class A and have its reference in struct Impl so Impl
functions can access it? (I am assuming from performance point of
view it should be the same as the code above).
Thanks
m.

Since struct A::Impl is a private data structure used only by A, you can do
whatever you want with it, including the trick that you're using right now.
No-one else can use the data structure, so no-one else would care what it
looks like.

-dr

 

[Grizlyk]

int Member1() const { return mrMember; }
int Member2() const;

int A::Member2() const { return mpImpl->mMember; }

The member functions Member1() and Member2() should return the same
value at all times, except for the Member1() operation can be inlined
while Member2() cannot.

Member2() can be inlined, if you are writing "inline" keyword

int Member1() const { return mrMember; }
inline int Member2() const;

inline int A::Member2() const { return mpImpl->mMember; }

Does it make sense for certain members, which are hidden in sturct
Impl, to expose references in the header file so the accessor
operations can be inlined?
Is it better, from the design point of view, to have the member
attribute in class A and have its reference in struct Impl so Impl
functions can access it? (I am assuming from performance point of
view it should be the same as the code above).

Not sure wah are speaking about, becasue "sturct A::Impl" is undefined.

 

[red floyd]

Member2() can be inlined, if you are writing "inline" keyword

int Member1() const { return mrMember; }
inline int Member2() const;

inline int A::Member2() const { return mpImpl->mMember; }

You don't want to do this, as it defeats the purpose of the pImpl,
namely reducing coupling. Otherwise, you need to have the definition of
the Impl class available. A good pImpl implementation should not inline
because you can then hide the implementation of the pImpl...

e.g.:

//MyClass.h

class MyImpl;
class MyClass
{
private:
MyImpl* pImpl;
public:
MyClass();
~MyClass();
void Method1();
// etc.
};

With this, clients don't need access to the definition of MyImpl. By
inlining, you will need to include "MyImpl.h" as well. Bad design, as
it increases unnecessary coupling (clients of MyClass don't *need* to
know what MyImpl looks like).

 

[Dave Rahardja]

Member2() can be inlined, if you are writing "inline" keyword

int Member1() const { return mrMember; }
inline int Member2() const;

inline int A::Member2() const { return mpImpl->mMember; }

No, it can't. In the OP's post, the definition of A::Member2() const is in the
..cpp file, not in the .h file.

 

[Dave Rahardja]

You don't want to do this, as it defeats the purpose of the pImpl,
namely reducing coupling. Otherwise, you need to have the definition of
the Impl class available. A good pImpl implementation should not inline
because you can then hide the implementation of the pImpl...

The OP has already done exactly what you suggested; check the original post.
struct A::impl was declared but not defined in the header file. It is defined
as a local structure in the source file.

-dr

 

[mliptak]

Member2() can be inlined, if you are writing "inline" keyword

int Member1() const { return mrMember; }
inline int Member2() const;

inline int A::Member2() const { return mpImpl->mMember; }

No it can't.
Either you mentioned to implement the Member2() in the .h file which I
can't and won't do because Impl is only declared in .h file. Or you
mentioned to implement the Member2() in the .cpp file. However when
calling Member2 from a different compilation unit, you get 'undefined
reference' error by linker, since the inlined function must be in
the .h file (check c++ FAQ lite [9.7]).

Not sure wah are speaking about, becasue "sturct A::Impl" is undefined.

Ok, what I meant is that you expose some of the Impl members in the
parent class (class A in my example) via references.

 

[mliptak]

You don't want to do this, as it defeats the purpose of the pImpl,

The OP has already done exactly what you suggested; check the original post.
struct A::impl was declared but not defined in the header file. It is defined
as a local structure in the source file.

Exactly. There's no question about whether the Impl should be visible
to "outside" world, because that's what pimpl is about. The question
is about the style when some of the members of Impl - especially those
which don't present additional compilation dependencies and are
accessed many times - are exposed in A's declaration in order for some
operations to get inlined.