[Amarok] 7bca886: debug: use elapsed timer

[Kevin Funk]

Tuesday 09 November 2010, "Rick W. Chen" <stuffcorpse at archlinux.us>:
> On 09 Nov 2010 11:47 +0100, Kevin Funk:
> > Tuesday 09 November 2010, "Rick W.Chen" <stuffcorpse at archlinux.us>:
> > > commit 7bca88681ee8248741d0b6daa966326512621bbd
> > > branch master
> > > Author: Rick W. Chen <stuffcorpse at 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