[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-optimize
Subject:    Re: QCString construction
From:       David Faure <faure () kde ! org>
Date:       2007-02-10 23:11:52
Message-ID: 200702110011.52373.faure () kde ! org
[Download RAW message or body]

On Saturday 10 February 2007, Roger Larsson wrote:
> On Saturday 10 February 2007 02:16, David Faure wrote:
> > 
> > One for Qt itself: why does QCString use the slow way when duplicating
> > QCStrings? (I know that QCString tmp( s1 ); is shallow copy, but the next
> > line calls operator+= which calls detach) Unless someone spots a flaw in
> > this patch I'll be posting it to TT (and I'm testing my local kde with it
> > starting from tomorrow ;)
> 
> I think it is because all these will require detach, so a new string has to be
> allocated anyway.
Can't see the relation. Yes a new string is allocated anyway, but in both cases
it won't make room for the added string before operator+ is called -- read the code, \
man ;)

> But will it be large enough to avoid an extra copy when appending?
> The tmp constructors could use a hint that they will grow.
> That could be done in the append anyway...
They could. But that's kind of the next step.

> Have you benchmarked that your Qt patch really is faster?
I think my other benchmark proves quite well that qstrlen is at fault,
comparing the two ways of doing string copying. But since you want proof,
here it is:

inline const QCString initialOperatorPlus( const QCString &s1, const QCString &s2 )
{
    QCString tmp( s1.data() );
    tmp += s2;
    return tmp;
}

inline const QCString myOperatorPlus( const QCString &s1, const QCString &s2 )
{
    QCString tmp( s1 );
    tmp += s2;
    return tmp;
}

[...]

   QCString astr = charData;
   QCString bstr = charData;
   time.restart();
   for ( uint i = 0; i < numIterations; ++i ) {
        QCString cstr = initialOperatorPlus(astr, bstr);
        assert( cstr[0] == 'A' );
        assert( cstr[dataLength-1] == 'A' );
        assert( cstr[2*(dataLength-1)] == '\0' );
   }
   qDebug( "Initial operator+ took %i milliseconds", time.elapsed() );

   time.restart();
   for ( uint i = 0; i < numIterations; ++i ) {
        QCString cstr = myOperatorPlus(astr, bstr);
        assert( cstr[0] == 'A' );
        assert( cstr[dataLength-1] == 'A' );
        assert( cstr[2*(dataLength-1)] == '\0' );
   }
   qDebug( "My operator+ took %i milliseconds", time.elapsed() );

Results:
Initial operator+ took 956 milliseconds
My operator+ took 731 milliseconds
  (... still slow because QCString::operator+=( const char *str ) is called, which \
calls qstrlen(str), but I can't avoid that one; stupid qcstring doesn't store its own \
length.) Yet, it is a 23% speed improvement already.

-- 
David Faure, faure@kde.org, sponsored by Trolltech to work on KDE,
Konqueror (http://www.konqueror.org), and KOffice (http://www.koffice.org).
_______________________________________________
Kde-optimize mailing list
Kde-optimize@kde.org
https://mail.kde.org/mailman/listinfo/kde-optimize


[prev in list] [next in list] [prev in thread] [next in thread]