From kde-core-devel  Mon Nov 05 16:24:30 2007
From: Philippe Fremy <phil () freehackers ! org>
Date: Mon, 05 Nov 2007 16:24:30 +0000
To: kde-core-devel
Subject: Re: [PATCH]: KIconLoader::loadIcon() speed ups
Message-Id: <472F43BE.70508 () freehackers ! org>
X-MARC-Message: https://marc.info/?l=kde-core-devel&m=119427998525445

Dan Meltzer wrote:
> On 11/5/07, Aaron J. Seigo <aseigo@kde.org> wrote:
>> hi all..
>>
>> attached is a patch that rather dramatically improves the speed of
>> KIconLoader::loadIcon().
>>
>> i felt like playing with code tonight, but on something other than plasma (in
>> an attempt to stave off getting burnt out dealing with QGV madnesses[1]) ...
>> so when i saw a bug report from Hamish on KIconLoader performance[2] i fired
>> up my text editor and got kcachegrind ready.
>>
>> the main culprits were KIconLoaderPrivate::removeIconExtension which was doing
>> this a little suboptimally but more importantly had a debug statement that's
>> probably superfluous at this stage in devel. off it went, and that revealed
>> the next horror shows.
>>
>> it was using QDir::isRelativePath exactly once and that was now accounting for
>> fully 33.46% of execution time. that's because it creaes a QFileInfo which
>> init's a file engine ...... yeah. way too much for a hot path like loadIcon.
>> i figure we don't -really- care if it's an absolute or relative path and we
>> can fudge it by asking if the first character is '/'.
>>
>> i'm going to out on a limb and guess that this breaks on windows. if so, i can
>> wrap it in an ifdef because the win is just way too big to ignore.
> 
> how about something like this to make them win folks happy?
> ..Completely untested of course, and I don't know what the exact
> specifications on win drive names are.. but I think they are char + :
> + \ ? is / possible for the third character? if not the check can be
> simplified more.
> 

Looking at the Qt code, a windows filename can be considered absolute if
it looks like:
- letter + ':' + '/' + path
- '//' + path

That's assuming of course that '\' have been converted to '/' already.

	cheers,

	Philippe