From kfm-devel Sat Sep 21 09:12:48 2013 From: David Faure Date: Sat, 21 Sep 2013 09:12:48 +0000 To: kfm-devel Subject: Re: UDSEntry compression ideas Message-Id: <8592441.Gn3TBs0gVr () asterix> X-MARC-Message: https://marc.info/?l=kfm-devel&m=137975442807965 On Friday 20 September 2013 00:32:08 Frank Reininghaus wrote: > Now the obvious question is: "How on earth can this be more efficient > that the current solution? There is still a QHash in each > UDSEntryPrivate after all, and the code gets more complicated!" > > The trick is that the mapping from the "uint uds" to the index of the > corresponding "Field" is usually exactly the same for every UDSEntry > if we read many of them in a row, and we can make all UDSEntryPrivates > share this mapping (QHash is implicitly shared). This means that every > UDSEntryPrivate only has a private QVector, which only needs a > rather small overhead compared to the memory that the Fields > themselves need. > > The patch definitely needs further testing and polishing. I probably > won't get much work done until next week though. Nice tricks, very clever. The patch looks good to me. Just some minor issues with the patch: * contains + value means double lookup, use iterators instead (like the code was doing before - with a typedef for the QHash, for readability) * the initialisation of cachedStrings can be simplified with QVector::resize which does call the default constructor for each item. Mark: you're right that in general when using QHash one has to remember the time of the hash calculation -- like my first QHash back when KUrl's hash value calculation needed to concatenate the parts of the url first (ouch!!!). However in the case of an int as key, the calculation is trivial (just a modulo). -- David Faure, faure@kde.org, http://www.davidfaure.fr Working on KDE, in particular KDE Frameworks 5