[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kfm-devel
Subject:    Re: UDSEntry compression ideas
From:       Frank Reininghaus <frank78ac () googlemail ! com>
Date:       2013-10-20 21:40:09
Message-ID: CAFoZWWgyf7YyAvvL_TGuPp6qxf8vJMt4bQh9oSdied+u9KMRWg () mail ! gmail ! com
[Download RAW message or body]

Hi,

2013/9/30 David Faure:
> On Thursday 26 September 2013 22:48:28 Frank Reininghaus wrote:
>> (a) With my patch, UDSEntry::remove(uint field) removes the "field"
>> from the QHash, but it neither frees the memory held by the
>> corresponding QString, nor does it remove the entry from the
>> QVector<Field>. I'm not sure how often it happens that fields are
>> removed and re-added to a UDSEntry, but if that is done repeatedly,
>> then the memory usage will grow indefinitely, which is not good.
>> Erasing the "Field" from the QVector every time might not be optimal
>> either because it makes "remove" a O(N) operation. Maybe one could
>> check if at least half the entries have been deleted and reorganize
>> the internal structure if that is the case. I'll look into that.
>
> I wouldn't worry too much about it. AFAICS remove is never called.
> (I removed remove - haha - locally and all of kio, kfile, kioslaves, libkonq
> and dolphin compiled just fine).

Maybe remove could be removed from the frameworks branch then ;-)

>> (b) Maybe I should add a unit test that stores multiple UDSEntries to
>> a stream, changes the order of the fields every time, re-loads them,
>> and verifies that everything is OK. Just to make sure that the code
>> really works even if not all UDSEntries have the same format inside
>> the stream.
>
> Sounds good. Don't use random though, to "change the order of the fields every
> time", since that would lead to errors that would be very hard to reproduce.

Did that, fixed a bug that was uncovered by the tests, and changed a
few more little things:

https://git.reviewboard.kde.org/r/113355/

Cheers,
Frank
[prev in list] [next in list] [prev in thread] [next in thread]