> On Feb. 13, 2014, 9:31 p.m., Christoph Feck wrote: > > Making the type movable does not make QList store it directly, how did you > > check this? > > > > http://qt-project.org/doc/qt-5/qlist.html says: > > > > "Internally, QList<T> is represented as an array of pointers to items of > > type T. If T is itself a pointer type or a basic type that is no larger > > than a pointer, or if T is one of Qt's shared classes, then QList<T> stores > > the items directly in the pointer array." > > Christoph Feck wrote: > Wait, UDSEntry is just a pointer, right?
Yes, it is a pointer to a UDSEntryPrivate. Thanks for bringing this issue up! - Frank ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115739/#review49739 ----------------------------------------------------------- On Feb. 13, 2014, 8:23 p.m., Frank Reininghaus wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/115739/ > ----------------------------------------------------------- > > (Updated Feb. 13, 2014, 8:23 p.m.) > > > 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
