Review Request: KImageCache optimization

Thu Feb 23 23:33:23 GMT 2012

> On Feb. 23, 2012, 8:56 p.m., Christoph Feck wrote:
> > kdeui/util/kimagecache.cpp, line 106
> > <http://git.reviewboard.kde.org/r/104052/diff/2/?file=50867#file50867line106>
> >
> >     This fails to correctly handle palettized images.
> >     
> >     If we do not want to preserve image texts, gamma, and DPI, please document it (the PNG handler did).
> >     
> >     Also, did you try setting the compression level for the PNG writer before giving up on compression? The compression is probably wanted. I have not checked if Qt uses quality 0 or 100 for the fastest mode, but the default PNG compression is utterly slow (as you noticed ;)
> 
> Mark Gaiser wrote:
>     Don't know about that.. Hope someone more knowledgeable could respond in the "image texts, gamma, and DPI".
>     
>     As for the fastest/slowest mode. No, i haven't tried that. But even then it seems kinda strange to load an image and directly save the exact same image as .png in some mmapped file (which was the behavior). Saving the "compiled" image (the bits) seems more obvious.
> 
> Christoph Feck wrote:
>     The KImageCache is not used exclusively for "loaded" images, but also (and especially so) for generated images, e.g. when caching SVG rendered data.
>     
>     Regarding compression, the type of images that are stored in the cache are usually very compressible, better than text, and thus the memory (or even disk) pressure can be greatly reduced. For example, the 128x128 "text-x-generic" icon is 64 KB uncompressed, while only 13 KB compressed.
>     
>     I just checked Qt docs. If you use "image.save(buffer, "PNG", quality);" you get uncompressed PNG with quality == 100, and fastest, but moderate compression with quality == 80. Any smaller value improves compression rate, but makes it slower.

I played a bit with compression and got it working.
The cached sizes (with qCompress(data, 5)) are indeed _way_ smaller then uncompressed. However, it crashes quite often when i use that. Here is the raw code for it: http://paste.kde.org/428102/

The unit tests pass on it yet kwin crashes on the image with this key: "15_1680_1015_hover-_widgets/viewitem". I haven't done further tests.
Compression rate is quite high:

key: "8_8_/usr/share/apps/desktoptheme/default/widgets/viewitem.svgzhover-bottomleft" 
cachedData size (compressed): 151 
cachedData size (uncompressed): 256 
key: "8_8_/usr/share/apps/desktoptheme/default/widgets/viewitem.svgzhover-bottomright" 
cachedData size (compressed): 149 
cachedData size (uncompressed): 256 

However, zlib is slow. If we really desperately want to have compression then we should go for something fast even if it doesn't compress as good: http://code.google.com/p/lz4/ The speed is about 20x faster with LZ4 then it would be with zlib but obviously with a lower ratio. Please do note that adding LZ4 is not hard, it's only 2 files (1 .c and 1 .h)

What i do find strange is that the file sizes of the caches seem to remain the same with and without compression. Yes, i do delete them prior to testing.

I leave it up to you guys where we go from here. I'm fine with adding compression (nice new interesting challenge ^_^), but i'm also fine with leaving it the way it's now in the diff + the edit from Aaron J. Seigo.

- Mark

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104052/#review10851
-----------------------------------------------------------

On Feb. 23, 2012, 7:23 p.m., Mark Gaiser wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104052/
> -----------------------------------------------------------
> 
> (Updated Feb. 23, 2012, 7:23 p.m.)
> 
> 
> Review request for kdelibs, David Faure and Michael Pyne.
> 
> 
> Description
> -------
> 
> I was running KWin through callgrind to see where possible bottlenecks are. I wasn't expecting much since it improved greatly during the 4.8 dev cycle, however one stood out. The saving of PNG images was taking about 1/5th of the time in KWin that i could see directly. That looked like something i might be able to optimize.
> 
> What this patch is doing is storing the actual image bits to prevent saving a PNG image to the mmapped cache. That was a hot code path in time (cycles), not even in calls. I've also reduced the amount of memory copies to a bare minimum by adding a rawFind function to KSharedDataCache which fills a QByteArray::fromRawData thus preventing a expensive memory copy. The rawFind is used for looking up an image and fetching it's data without copying it. That is done because QImage seems to make a copy itself internally. I don't have any performance measurements, however, prior to this patch my kwin test was using up ~5.000.000.000 cycles. After this patch it's using up 1.370.000.000. I don't have raw performance numbers to see if the cache itself is actually faster, it certainly has become a lot cheaper to use the cache. Logic wise i would say creating a QImage from the cached data should be way faster now since there is no step involved anymore in decoding the image. Storing is certainly an order of magnitude faster.
> 
> Benchmark numbers. insert(write) and find(read)
> 
> -- Before patch --
> READ : 0.019 msecs per iteration (total: 79, iterations: 4096)
> WRITE: 0.010 msecs per iteration (total: 88, iterations: 8192)
> 
> -- After patch --
> READ : 0.019 msecs per iteration (total: 79, iterations: 4096)
> WRITE: 0.0026 msecs per iteration (total: 87, iterations: 32768)
> 
> Reading is equal in speed, writing is ~5x faster after the patch.
> 
> Special thanks go to David Faure for helping me a great deal with this.
> 
> 
> Diffs
> -----
> 
>   kdecore/util/kshareddatacache.h 339cecc 
>   kdecore/util/kshareddatacache.cpp 9fe3995 
>   kdeui/tests/CMakeLists.txt 63788f6 
>   kdeui/tests/kimagecachetests.h PRE-CREATION 
>   kdeui/tests/kimagecachetests.cpp PRE-CREATION 
>   kdeui/util/kimagecache.cpp a5bbbe1 
> 
> Diff: http://git.reviewboard.kde.org/r/104052/diff/
> 
> 
> Testing
> -------
> 
> I've also written a bunch of test cases (greatly improved by David Faure) to see if i didn't break anything. According to the test (which is also comparing the actual image bits) it's all passing just fine.
> 
> 
> Thanks,
> 
> Mark Gaiser
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20120223/8dedc5a3/attachment.htm>