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

List:       koffice-devel
Subject:    Re: koffice/plugins/pathshapes/enhancedpath
From:       Thomas Zander <zander () kde ! org>
Date:       2010-10-19 12:47:42
Message-ID: 201010191447.42191.zander () kde ! org
[Download RAW message or body]

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!

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;
>          }

-- 
Thomas Zander
_______________________________________________
koffice-devel mailing list
koffice-devel@kde.org
https://mail.kde.org/mailman/listinfo/koffice-devel
[prev in list] [next in list] [prev in thread] [next in thread]