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