Review Request: Plasma::Svg: Do not require exact match for size hinted elements.

[Ingomar Wesp]

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20101025/15b9608c/attachment-0001.htm