Review Request 119425: Cache the textures created for the fast path

[Aleix Pol Gonzalez]

> On July 24, 2014, 10:55 a.m., David Edmundson wrote:
> > src/declarativeimports/core/framesvgitem.cpp, line 45
> > <https://git.reviewboard.kde.org/r/119425/diff/2/?file=292260#file292260line45>
> >
> >     Having spoken to you offline this does make sense, but it needs documentation as to why it's doing this.
> 
> Marco Martin wrote:
>     uhm, why a filter on palette change?
>     this would work for themes that use system colors but not the other ones.
>     theme::themechanged should cover both cases (and if it doesn't, it should)
> 
> Aleix Pol Gonzalez wrote:
>     See SvgPrivate::checkColorHints().
>     
>     I agree it could make sense having it in plasma theme, but this should be part of another patch.
> 
> Marco Martin wrote:
>     it does a repaintneeded, that is correct.
>     but yeah, having it in theme doesn't make much sense, because technically the theme didn't change and you can't know from there if one of the svgs actually needs a repaint.
>     
>     so, what all of this suggests me is that the thing that would make most sense is actually use repaintneeded, and either:
>     a) just remove from the hash the id of this texture (multiple removes still possible tough)
>     b) event compress the clear()
>     c) both of the above

I'm getting a bit lost, sorry. Why is listening to all svg's better? RepaintNeeded will emit upon signals we don't need. Note that themeChanged doesn't clean the cache and it will rather be the textures just stopping to use the old theme, because the hash is refcounted. That's why I introduced more elements to the hash key.


- Aleix


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119425/#review63049
-----------------------------------------------------------


On July 24, 2014, 11:14 a.m., Aleix Pol Gonzalez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119425/
> -----------------------------------------------------------
> 
> (Updated July 24, 2014, 11:14 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> -------
> 
> Create a cache that has pointers to all the textures that we've generated, so in case we have one already created, we can re-use it.
> 
> 
> Diffs
> -----
> 
>   src/declarativeimports/core/framesvgitem.cpp 323b06b 
>   src/declarativeimports/core/iconitem.cpp 38012cc 
>   src/declarativeimports/core/svgitem.cpp eccff55 
>   src/declarativeimports/core/svgtexturenode.h 21b9b2f 
> 
> Diff: https://git.reviewboard.kde.org/r/119425/diff/
> 
> 
> Testing
> -------
> 
> see the qDebug (to be removed before commit). 
> 
> plasmashell 2> out
> $ grep s_cache out | grep ": miss" | wc -l
> 342
> $ grep s_cache out | grep ": hit" | wc -l
> 126
> 
> So still having 3 times more hits than miss, so there's big room for improvement. Good news is that with this, we get a ~25% of memory and bandwidth save, in a per-item basis.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20140724/32dcbb60/attachment.html>