> On 2010-10-25 08:03:51, Manuel Mommertz wrote:
> > If you want to use the DOM to find the size hints, you can take a look at 
> > KGameSvgDocument. It implements a way to find elements by id. This could 
> > should be usable for you.
> > 
> > And can you please measure how long it takes to load the SVG and to look 
> > for an element with and without your patch?

> And can you please measure how long it takes to load the SVG and to look for 
> an element with and without your patch?

Sure. Obviously, the following is not proper benchmarking, but maybe it can 
provide at least some insight into the performance implications.

For SharedSvgRenderer:load, I measured the total walltime spent in the method 
body during the startup of my plasma-desktop session with and without the 
patch. In both cases, SharedSvgRenderer::load was called 12 times during the 
startup. I made 5 measurements each and took the median value. 

On both my desktop and my netbook I noticed a slowdown to about 0.75 of the 
walltime the unpatched version took, which amounted to a median increase in 
plasma-desktop startup time of ~30 ms on my desktop and ~100 ms on my netbook. 
I also ran "plasma-desktop --nofork" through callgrind, but the cycle counts 
did't seem to relate to the measured walltime at all :/

Measuring SvgPrivate::findInCache is probably hard to do in a meaningful way, 
since most SVGs don't use size hinted elements, in which case the impact is a 
single lookup into an empty hashtable as well as an additional call to 
createRenderer(). If there are elements with size hints, the search for the 
best fit scales linearly with the number of size hinted elements with the given 
base element id. Since even then it doesn't do very much, this should not 
really impact performance in any noticeable way. Just for the sake of it, I did 
a few measurements anyway, but the impact was too minimal to provide meaningful 
results (createRenderer() and the search for the best of the 6 candidates takes 
an average of little more than 1 µs when put into a loop, but obviously this 
has a far better caching behavior than a normal invocation)


- Ingomar


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/5689/#review8338
-----------------------------------------------------------


On 2010-10-25 00:17:45, Ingomar Wesp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/5689/
> -----------------------------------------------------------
> 
> (Updated 2010-10-25 00:17:45)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> Previously, if an SVG contained size hinted elements, they were only used 
> when the display size matched the size hint exactly. This patch tries to 
> relax this condition by searching for the smallest size hinted element that 
> is still bigger than the display size (in order for the element to be chosen, 
> it also has to have the same aspect ratio). If no such element can be found, 
> it falls back to the normal element id as passed.
> 
> In order to speed up the lookup (and because it appears to be impossible to 
> access the DOM of an already loaded SvgRenderer), all size hinted element ids 
> are stored in SharedSvgRenderer at load time.
> 
> I think it would be good to change the QRegExp based id fetching into a 
> proper DOM traversal. Are there any convenience functions in KDELibs that 
> allow easy iterating over all elements (couldn't find any) or do I have to 
> implement that myself based on Qt's DOM classes?
> 
> Please tell me what you think... Have I missed something?
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/plasma/private/svg_p.h 1189389 
>   /trunk/KDE/kdelibs/plasma/svg.cpp 1189389 
> 
> Diff: http://svn.reviewboard.kde.org/r/5689/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ingomar
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to