[PATCH] KFileItem performance improvement

[David Faure]

On Friday 27 June 2008, Peter Penz wrote:
> Hi David,
> 
> [...]
> > > I think it's a good idea, but as with all caches, the main problem is
> > > when to invalidate the cache. 1) if you rename foo.txt to foo.jpg the
> > > icon used to change, but doesn't change  anymore, right? Or 2) if you use
> > > the properties dialog to change the icon of a .desktop file,
> [...]
> > I'll take a look on those 2 issues (I won't have time until mid of next
> > week for KDE -> I'll reply until the end of next week).
> 
> I had some unexpected time today -> I could solve issues 1. and 2. by the 
> attached patch. You've been right, it's not that trivial as I initially 
> thought. The cached version of the icon name may only be used _after_ the 
> mime type is known, otherwise usecase 2. won't work at all.
> 
> Usecase 1. could be solved by clearing the cache in 
> KFileItem::refreshMimeType(). As it cannot be seen very clear in the patch - 
> m_iconName is cleared in:
> - KFileItem::refreshMimeType()
> - KFileItemPrivate::readUDSEntry()
> - KFileItem::setUDSEntry()

OK.

> I verified again whether the caching effect is still available and the results 
> are very good (the mime type usually is available very early). I'm still 
> amazed how often KFileItem::iconName() is invoked. E. g. hovering a file 
> results in a lot of calls, as the icon is required for each step of the 
> fade-in animation.

(Maybe the animation code could cache some data too? ;)

> One note to usecase 2.: when changing the icon in the properties dialog, first 
> a "binary data" icon is shown. Only after hovering the item, the correct icon 
> is shown. This issue also occured _before_ my patch and must be fixed 
> seperately. I have some more infos regarding this issue, but I think we 
> should discuss this in a different thread.

Hmm, OK.

> > > 3) is the really hard one I think. There is no direct relation between
> > > "ksycoca has new definition
> > > for this mimetype" and "we need to clear the cache in all kfileitems".
> 
> I did not check this usecase yet, as - like you said - I think we can rely 
> here on a reload by the user and does not work yet also without caching.
> 
> Any thoughts?

Great work as always, thanks. Please commit.

I'll consider the lack of unit test as a TODO item for me.

-- 
David Faure, faure at kde.org, sponsored by Trolltech to work on KDE,
Konqueror (http://www.konqueror.org), and KOffice (http://www.koffice.org).