Review Request 124811: KIconLoader: speed up hasIcon/iconPath by using the on-disk cache from loadIcon

[Mark Gaiser]

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


Hi David,

This probably makes it faster (i have no doubt there), but i don't really follow how this works.
Could you perhaps explain what you're doing exactly here?

For instance, how can you determine if something is cached based on just an int increment?
What "would" make more sense to me is using a bool, for instance this:
KICONTHEMES_EXPORT bool kiconthemes_unittests_searchOnDisk = false;
which would be set to false upon entering the iconPath() function (to always make it false by default) and would be set to true when if (d->findCachedPixmapWithPath(key, Q_NULLPTR, path) && !path.isEmpty()) { ... } eveluates to true.

But then again, i could be missing the clue here :) (likely)

- Mark Gaiser


On aug 19, 2015, 7:25 a.m., David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124811/
> -----------------------------------------------------------
> 
> (Updated aug 19, 2015, 7:25 a.m.)
> 
> 
> Review request for KDE Frameworks, Christoph Feck and Volker Krause.
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> -------
> 
> This makes QIcon::fromTheme() much faster, for all cases where we have loaded the
> icon once before (including in another process).
> 
> 
> Diffs
> -----
> 
>   autotests/kiconloader_unittest.cpp 6b60e7ebfc970c94ae865d56c4e33a8034b4fcee 
>   src/kiconloader.cpp db3aa8f8fd6f8fb706edcb27cce073c36831934d 
> 
> Diff: https://git.reviewboard.kde.org/r/124811/diff/
> 
> 
> Testing
> -------
> 
> The unittest has a way to see if KIconLoader used the cache or searched on disk, with the newly exported global int. I couldn't think of a better way.
> 
> I'll let Volker do the real-world testing and measure the overall impact as he did previously.
> 
> 
> Thanks,
> 
> David Faure
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150819/9966324d/attachment.html>