-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118452/
-----------------------------------------------------------

(Updated Dec. 28, 2014, 10:22 p.m.)


Status
------

This change has been marked as submitted.


Review request for KDE Frameworks and David Faure.


Repository: kio


Description
-------

I am continuing to split up https://git.reviewboard.kde.org/r/113355/ , which 
attempts to make UDSEntry more efficient memory and CPU-wise, into independent 
parts. This is the third step after 
https://git.reviewboard.kde.org/r/113591/ and 
https://git.reviewboard.kde.org/r/115739/ .

The present patch modifies the internal data storage of UDSEntry. UDSEntry 
contains a mapping from unsigned int keys to "Field" values, where Field is a 
struct that contains a QString and a long long (some keys correspond to numeric 
values, like size, date, etc, whereas others, like user and group, correspond 
to a QString).

Currently, UDSEntry stores all data in a QHash<uint, Field> internally. This 
ensures that everything can be accessed in O(1) time, but is not very efficient 
memory-wise because a separate memory allocation is done for each hash node.

I propose to change this and store both the uint keys and the Field values in a 
QVector each. This means that accessing a value becomes a O(n) operation, since 
the entire QVector of keys may need to be scanned in order to find a value, but 
because the number n of values in a UDSEntry is usually quite small and can 
currently not exceed a number ~100, this should not matter in practice.

Some parts of https://git.reviewboard.kde.org/r/113355/ are still missing:

(a) The QVectors which store the keys (which are usually the same for all items 
in a directory) are not shared yet. Changing this would reduce the memory usage 
further, but I decided to keep this change separate in order to keep the 
current patch small and easy to understand. Moreover, this makes it easier to 
benchmark other similar approaches (e.g., replace QVector by std::vector, or 
store keys and values together in a std::vector<std::pair<uint,Field>>).

(b) No space is reserved in the vectors when key/value pairs are inserted one 
by one. Implementing this would make UDSEntry faster on the slave side (since 
repeated re-allocations would not be necessary any more), but this can be done 
in a later patch. Moreover, it might not be needed any more if UDSEntry is not 
used directly any more on the slave side, as suggested on the frameworks 
mailing list by Aaron (good idea BTW!). 


Diffs
-----

  tests/udsentrybenchmark.cpp 9bedb7b 
  autotests/udsentry_benchmark.cpp b5fa8d6 
  src/core/udsentry.h 98a7035 
  src/core/udsentry.cpp b806e0e 
  src/ioslaves/file/file.cpp 1a2a767 

Diff: https://git.reviewboard.kde.org/r/118452/diff/


Testing
-------

Unit tests still pass.

The memory usage of listjobtest with a directory with 100,000 files is reduced 
from 71344 K to 35392 K according to KSysGuard. I see similar savings when 
opening the directory in Dolphin.

I still haven't set up a Qt5/KF5 build in release mode (shame on me!), so I 
cannot present any benchmark results.


File Attachments
----------------

Benchmark results
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/12/09/038e443c-78eb-443b-b33a-b451b28d10ea__UDSEntry-benchmarks.png


Thanks,

Frank Reininghaus

_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Reply via email to