[KConfigGroup] reserve() more and add some C++11
ClosedPublic

Authored by broulik on Sep 19 2017, 2:07 PM.

Details

Summary

Using initializer_lists for QList we reserve the right amount of memory in advance and also make for nicer code.
Also uses range-for where code as touched and a const container was used.

Test Plan

Test still pass.

asRealList is called 4200x on plasmashell startup for me, saves 2ms for me

Diff Detail

Repository
R237 KConfig
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
broulik created this revision.Sep 19 2017, 2:07 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptSep 19 2017, 2:07 PM
Restricted Application added a subscriber: Frameworks. · View Herald Transcript
davidedmundson accepted this revision.Sep 19 2017, 2:33 PM
This revision is now accepted and ready to land.Sep 19 2017, 2:33 PM
mwolff requested changes to this revision.Sep 19 2017, 3:21 PM
mwolff added a subscriber: mwolff.

it would probably be a good idea to rewrite KConfigGroupPrivate::serializeList to not take a QVariantList, but rather to use a streaming API. I.e. instead of:

void foo(myList)
{
    varList = convertList(myList);
    write(KConfigGroupPrivate::serializeList(list));
}

Do something like:

void foo(myList)
{
    var value = KConfigGroupPrivate::serializeList(mylist)
    write(value);
}

where serializeList is a template that does $magic internally to convert a list of values to a serializeable format (i.e. iterate over values, then stream them into QDataStream/QTextStream, wrap in QVariant only when needed). Note that you can then use initializer_list for the "static" lists like for QPoint et al.

src/core/kconfiggroup.cpp
185

can you make this (and the QList<qreal> below) a QVector instead? Would save 50% of memory on 64bit machines

187

use splitRef instead, also below

This revision now requires changes to proceed.Sep 19 2017, 3:21 PM
broulik planned changes to this revision.Sep 20 2017, 6:12 PM
broulik added inline comments.
src/core/kconfiggroup.cpp
187

Was my first reflex do to so but string confusingly is a QByteArray

mwolff accepted this revision.Sep 21 2017, 8:39 AM

ah ok, and since we don't have a proper byte array view yet, there's nothing you can do (which is sad here!)

the other comments from me where suggestions for further improvements, they don't need to hold up this patch

This revision was automatically updated to reflect the committed changes.