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

(Updated Feb. 27, 2014, 7:52 a.m.)


Status
------

This change has been marked as submitted.


Review request for KDE Frameworks and David Faure.


Repository: kio


Description
-------

I'm continuing my efforts to make UDSEntry more efficient, which were started 
in https://git.reviewboard.kde.org/r/113591/. This is the second step, and I'll 
probably do more in the future, for which I will split 
https://git.reviewboard.kde.org/r/113355/ into independent parts.

This patch contains the following changes (which are separate commits):

1. Make UDSEntry a Q_MOVABLE type. This has the effect that a QList<UDSEntry> 
a.k.a. UDSEntryList will store the actual entries in a single allocated block 
of memory, and not pointers to UDSEntries which are allocated individually on 
the heap (this means that this change is binary incompatible). This reduces the 
memory usage by 32 bytes per UDSEntry in a QList because each memory allocation 
uses at least 32 bytes on a 64-bit system.

This idea is similar to what I proposed for KFileItem in 
https://git.reviewboard.kde.org/r/111789/. That one had to be reverted later 
though because it turned out that KDirLister does rely on QList<KFileItem> 
storing only pointers to the KFileItems. I'm confident that no such trouble 
will happen for UDSEntry - all KIO unit tests still pass.

One could argue that we could simply use a UDSEntryVector instead of a 
UDSEntyList to achieve the same memory savings, but considering that the list 
is used quite frequently ( http://lxr.kde.org/ident?i=UDSEntryList ), this 
might require a lot of porting work and cause other unexpected problems.

Note that I could not make the Q_DECLARE_TYPEINFO declaration work if it was 
inside the KIO namespace, but I still preferred to do it immediately after the 
class declaration, so I had to interrupt the namespace briefly.

2. Add some benchmarks to measure how long typical UDSEntry operations take: 
add fields to an entry, read fields from an entry, store a UDSEntryList in a 
QByteArray, and read it back from the QByteArray.

All measurements are done both for UDSEntries with 8 fields (this is a rather 
typical use case because kio_file usually creates 8 fields), and for entries 
with the maximum number of fields.

3. Add a simple manual test ('listjobtest') that runs a KIO::ListJob for a 
given URL. This can be used to easily measure the memory usage of the 
UDSEntryList that contains an entry for each file at that URL.


Diffs
-----

  src/core/udsentry.h 9550a7e 
  tests/CMakeLists.txt dbca6a5 
  tests/listjobtest.cpp PRE-CREATION 
  tests/udsentrybenchmark.cpp PRE-CREATION 

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


Testing
-------

1. KIO unit tests still pass.

2. I've confirmed with the new 'listjobtest' and KSysGuard that the memory 
usage is really lowered by 32 bytes per item in a UDSEntryList.

3. The benchmark results do not change significantly. I only tested it with a 
debug build (i.e., with optimizations disabled) though, and I'm afraid I might 
be lacking the resources to set up an additional build of Qt5 and all of KF5 in 
release mode. However, since UDSEntry essentially only depends on qtbase, I 
might be able to just do a release build of qtbase and build a stand-alone 
version of UDSEntry+benchmarks on top of that. I'll look into this option for 
getting reliable benchmark results when I work on further improvements in the 
future.


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