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

List:       kde-commits
Subject:    Re: koffice/plugins/pathshapes/enhancedpath
From:       Carlos Licea <carlos.licea () kdab ! com>
Date:       2010-10-19 15:54:24
Message-ID: 201010190954.29980.carlos.licea () kdab ! com
[Download RAW message or body]


On Martes 19 Octubre 2010 06:47:42 Thomas Zander escribió:
> The work is good, just wanted to point out the unicode() optimization would
> not have been needed. Here some tips on optimizing Qt code :)
> 
> The  'unicode()' method is inline and there is not a compiler that will not
> essentially do exactly what you did manually. Naturally the compiler will
> only truely do this in optimized code.
> The main difference now is that you added a copy to the uni_ch whereas the
> compiler would have done a direct comparisone with the QChar's only member
> (essentially a static_cast<ushort>(myQChar))
> 
> For optimizing low level stuff like this you should always use optimized
> code and read the assembler generated or just run it with something like
> cachegrind to check the amount of cycles actually spent.
> Using cachegrind or similar can be done using the Qt testlib, so writing a
> benchmark for this before optimizing it will help you a lot on testing if
> your changes will have a measurable speedup.
> 
> In many cases obvious fixes may not actually help, and vice versa. So
> always measure!

Wouldn't you get the exact same behavior you describe if you declare uni_ch 
const? I would assume so as you can prove that, in essence, uni_ch is now a 
"reference" to the internal object (not in the C++ definition.) 

> On Tuesday 19. October 2010 10.51.42 Thorsten Zachmann wrote:
> > SVN commit 1187393 by zachmann:
> > 
> > o more speeding up of loading of enhanced path shapes.
> > 
> > - don't update the paths shape while it is loaded, only update once all
> > 
> >   command are in
> > 
> > - call unicode only once per character
> > 
> >  M  +10 -9     EnhancedPathShape.cpp
> > 
> > --- trunk/koffice/plugins/pathshapes/enhancedpath/EnhancedPathShape.cpp
> > #1187392:1187393 @@ -463,16 +463,17 @@
> > 
> >      bool separator = true;
> >      for (int i = 0; i < data.length(); ++i) {
> >      
> >          QChar ch = data.at(i);
> > 
> > -        if (separator && (ch.unicode() == 'M' || ch.unicode() == 'L'
> > -            || ch.unicode() == 'C' || ch.unicode() == 'Z'
> > -            || ch.unicode() == 'N' || ch.unicode() == 'F'
> > -            || ch.unicode() == 'S' || ch.unicode() == 'T'
> > -            || ch.unicode() == 'U' || ch.unicode() == 'A'
> > -            || ch.unicode() == 'B' || ch.unicode() == 'W'
> > -            || ch.unicode() == 'V' || ch.unicode() == 'X'
> > -            || ch.unicode() == 'Y' || ch.unicode() == 'Q')) {
> > +        ushort uni_ch = ch.unicode();
> > +        if (separator && (uni_ch == 'M' || uni_ch == 'L'
> > +            || uni_ch == 'C' || uni_ch == 'Z'
> > +            || uni_ch == 'N' || uni_ch == 'F'
> > +            || uni_ch == 'S' || uni_ch == 'T'
> > +            || uni_ch == 'U' || uni_ch == 'A'
> > +            || uni_ch == 'B' || uni_ch == 'W'
> > +            || uni_ch == 'V' || uni_ch == 'X'
> > +            || uni_ch == 'Y' || uni_ch == 'Q')) {
> > 
> >              if (start != -1) { // process last chars
> > 
> > -                addCommand(data.mid(start, i - start));
> > +                addCommand(data.mid(start, i - start), false);
> > 
> >              }
> >              start = i;
> >          
> >          }

-- 
Carlos Licea | carlos.licea@kdab.com | Software Engineer
Klarälvdalens Datakonsult AB, a KDAB Group company
Tel. Sweden (HQ) +46-563-540090, USA +1-866-777-KDAB(5322)
KDAB - Qt Experts - Platform-independent software solutions

["signature.asc" (application/pgp-signature)]

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