Review Request: KImageCache optimization

[Mark Gaiser]

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

(Updated Feb. 26, 2012, 8:08 p.m.)


Review request for kdelibs, David Faure and Michael Pyne.


Changes
-------

Revision 6.. rawFind removed. My initial test data was from kwin. That was filled with data from all over the place. I saw QImage::copy calls, but those where not from the KImageCache. The clean testcase that i've been using indicated that so a copy of the data has to be made for QImage which makes rawFind obsolete. The performance gets only a minor hit from this. The numbers as posted before still stand.


Description (updated)
-------

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.

-- update 26-02-2012 --
After spending a lot more time trying to get compression in the mix "someone" came with the suggestion to use KFilterDev. Never heard of it, but i gave it a shot anyway. It turned out to be the bull-eye in this case. Data is now greatly compressed using gzip (any other compression available in KFilterDev makes it a lot slower). Gzip gives some loss in speed, but compared to the current stock KImageCache it's still a _lot_ faster.

New benchmarks compared to the stock 4.8.0 KImageCache:
Read: ~8x faster (was 5x in my previous patch)
Write: ~5x faster (equally fast compared to stock, no speedup here in my previous patch)
Inserting the same image with the same key twice is now about free (depending on the size of the image, it's about free for icon sized images with 48x48).

I've also added a more detailed description of insertImage for the details that it saves. My point of view is that KImageCache should be used for caching of (system)images and perhaps thumbnails. Things like image text should not be stored since that should be fetched from the originating image. The color table (when available) is stored.

A last massive performance improvement that is possible but complicated to get is by using the IZ compression from "kdepepo". Just using IZ to compress the bits will make the image size a lot smaller then is currently done with gzip and will do it at the very least about 5x faster as well. That means insertion will benefit massively here. However, that will also mean that the stored image data is only going to be:
- width
- height
- format
- bits
No color tables! But this is something for the future. Right now the current diff is about fine. The last remaining issues are the possible race conditions as mentioned in the comments below. 
I need help in that area. I can't do what's suggested (i lack the knowledge to do it myself).

Special thanks go to David Faure for helping me a great deal with this.


Diffs (updated)
-----

  kdecore/util/kshareddatacache.h 339cecc 
  kdecore/util/kshareddatacache.cpp 9fe3995 
  kdeui/tests/CMakeLists.txt c8b8c85 
  kdeui/tests/kimagecachetests.h PRE-CREATION 
  kdeui/tests/kimagecachetests.cpp PRE-CREATION 
  kdeui/util/kimagecache.h 54112de 
  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/20120226/04d6fdf8/attachment.htm>