Tuesday 09 November 2010, "Rick W. Chen" <stuffcor...@archlinux.us>:
> On 09 Nov 2010 11:47 +0100, Kevin Funk:
> > Tuesday 09 November 2010, "Rick W.Chen" <stuffcor...@archlinux.us>:
> > > commit 7bca88681ee8248741d0b6daa966326512621bbd
> > > branch master
> > > Author: Rick W. Chen <stuffcor...@archlinux.us>
> > > Date: Tue Nov 9 17:06:12 2010 +1300
> > >
> > > debug: use elapsed timer
> > >
> > > as kdelibs SVN commit 1194322
> >
> > (snip)
> >
> > > @@ -130,6 +129,8 @@ using Debug::fatal;
> > >
> > > /// Performance logging
> > > #define PERF_LOG( msg ) { Debug::perfLog( msg, __PRETTY_FUNCTION__ );
> > > }
> > >
> > > +class BlockPrivate;
> > > +
> > >
> > > namespace Debug
> > > {
> > >
> > > /**
> > >
> > > @@ -157,13 +158,11 @@ namespace Debug
> > >
> > > class Block
> > > {
> > >
> > > public:
> > > - AMAROK_CORE_EXPORT Block(const char* name);
> > > + AMAROK_CORE_EXPORT Block( const char *name );
> > >
> > > AMAROK_CORE_EXPORT ~Block();
> > >
> > > private:
> > > - QTime m_startTime;
> > > - const char *m_label;
> > > - int m_color;
> > > + BlockPrivate *const d;
> > >
> > > };
> > >
> > > /**
> > >
> > > diff --git a/src/core/support/Debug_p.h b/src/core/support/Debug_p.h
> > > index 20ea59a..43d0676 100644
> > > --- a/src/core/support/Debug_p.h
> > > +++ b/src/core/support/Debug_p.h
> > > @@ -20,7 +20,12 @@
> > >
> > > #include "Debug.h"
> > >
> > > #include <QString>
> > >
> > > -#include <QTime>
> > > +
> > > +#if QT_VERSION >= 0x040700
> > > +# include <QElapsedTimer>
> > > +#else
> > > +# include <QTime>
> > > +#endif
> > >
> > > class IndentPrivate
> > >
> > > : public QObject
> > >
> > > @@ -37,11 +42,15 @@ public:
> > > class BlockPrivate
> > > {
> > >
> > > public:
> > > - BlockPrivate(const char* label);
> > > -
> > > - QTime m_startTime;
> > > - const char *m_label;
> > > - int m_color;
> > > + BlockPrivate( const char *text );
> > > +
> > > +#if QT_VERSION >= 0x040700
> > > + QElapsedTimer startTime;
> > > +#else
> > > + QTime startTime;
> > > +#endif
> > > + const char *label;
> > > + int color;
> > >
> > > };
> > >
> > > #endif // DEBUGPRIVATE_H
> >
> > Hey Rick,
> >
> > there was a reason for not using the pimpl idiom in the Block class. I
> > didn't add a comment about this, though.
> >
> > I have no performance benchmarks, but it could get notably slow if you're
> > operating on the heap for each ctor/dtor call of Block. We're already
> > using this in most parts of Amarok, this code is used extensively during
> > Amarok startup. Also this is still the case within release builds.
> >
> > Please do some benchmarks with regards to this, just to be sure. Startup
> > time is important.
>
> I noticed in Debug_p.h BlockPrivate was defined but not used, so I
> thought it was intended to be used later. I've not noticed slower
> startup times but yes not using the private ptr stuff could have an
> improvement. This only matters if debug is turned on though.
>
> What's a good way to benchmark this?
I'd say: Write a little test case which loops through some specific amount
(1000?) of debug() calls in nested functions with DEBUG_BLOCK defined.
One time with pimpl idom being used, one time without.
You could also add this to Amarok's test suite.
Q_BENCHMARK could be useful in this case.
--
Kevin Funk
_______________________________________________
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel
