Review Request 111560: Use QStringBuilder
Friedrich W. H. Kossebau
kossebau at kde.org
Thu Jul 18 12:19:41 BST 2013
> On July 18, 2013, 10:39 a.m., Boudewijn Rempt wrote:
> > Hm, I don't know... I doubt it actually makes a difference in most if not all of the cases -- for instance, changing the way a string is constructed for a dialog will not make the dialog pop up perceptibly faster, and I see very few if any critical path cases here. And in almost all cases there's more code than there was before, which makes for harder reading.
It's less about dialogs, but more about the filters. Or the query builders (calligradb, kexi, sheets, ...). In both cases lots of long consecutive additions of QStrings are done. And those definitely are on the hot paths.
Given the large codebase of Calligra I was surprised how few fixes I had to do for where the implicit conversion from QString did not work any longer, due to the additional one between QStringBuilder and QString. So it's not that much extra code, the big majority of QString places could stay unchanged.
I also have a follow-up patch which builds up on this one, where I turn multiple (x += ...) into a single x = ... + [...] + ..., where useful. 40+ places. And I did not yet go for all places, just some randoms one I found on grepping for +=
So think this is still worthwhile. No numbers yet. But it should be expected to improve things, given the saved mallocs. And for that it's really not that much extra code needed, in absolute terms.
- Friedrich W. H.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111560/#review36103
-----------------------------------------------------------
On July 18, 2013, 10:25 a.m., Friedrich W. H. Kossebau wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111560/
> -----------------------------------------------------------
>
> (Updated July 18, 2013, 10:25 a.m.)
>
>
> Review request for Calligra.
>
>
> Description
> -------
>
> There are lots of string concatenations all over, so using QStringBuilder might speedup things a little.
>
>
> Diffs
> -----
>
> 3rdparty/kdchart/CMakeLists.txt b7178e6
> CMakeLists.txt 4f38f33
> active/src/CAPADocumentModel.cpp f8c03d0
> active/src/CATextDocumentModel.cpp 296c964
> braindump/plugins/stateshape/StatesModel.cpp e847a34
> braindump/src/SectionsIO.cpp d5534fa
> filters/words/epub/OdtHtmlConverter.cpp 8aac224
> filters/words/mobi/OdtMobiHtmlConverter.cpp 679fcd9
> filters/words/msword-odf/conversion.cpp e161199
> kexi/main/startup/KexiRecentProjectsModel.cpp 6c5e078
> kexi/plugins/importexport/csv/kexicsvimportdialog.cpp a75d7cd
> kexi/plugins/queries/kexiquerydesignerguieditor.cpp 01de184
> kexi/widget/navigator/KexiProjectModelItem.cpp 6e50585
> krita/image/metadata/kis_meta_data_value.cc aa507cc
> krita/image/tests/kis_meta_data_test.cpp f520113
> krita/plugins/extensions/dockers/compositiondocker/compositiondocker_dock.cpp 81edeab
> krita/plugins/paintops/libpaintop/kis_pressure_hsv_option.cpp 957f927
> krita/sdk/tests/filestest.h d14e748
> krita/ui/kis_doc2.cc f18d718
> libs/db/RecordData.cpp 1152e74
> libs/db/connectiondata.cpp 23fb5ad
> libs/db/field.cpp ec9a7a9
> libs/db/queryschema.cpp 9c0f771
> libs/koreport/items/text/krscripttext.cpp dc1d4ab
> plan/libs/kernel/tests/CalendarTester.cpp eae353c
> plan/libs/kernel/tests/ProjectTester.cpp f3e837b
> plan/libs/kernel/tests/WorkInfoCacheTester.cpp 34b299e
> plan/libs/models/kptnodeitemmodel.cpp 60ed7bf
> plan/libs/models/kptpertcpmmodel.cpp 621f78a
> plan/libs/models/kptrelationmodel.cpp 1b60698
> plan/plugins/schedulers/rcps/tests/ProjectTester.cpp 26595f1
> plan/plugins/schedulers/tj/tests/SchedulerTester.cpp d447d14
> plan/plugins/schedulers/tj/tests/TaskJuggler.cpp a8474d6
> plan/plugins/scripting/Project.cpp b9d9ce0
> plugins/dockers/shapecollection/ShapeCollectionDocker.cpp d08ee37
> plugins/musicshape/core/tests/MusicXmlWriterTest.cpp ca9a005
> words/part/KWDocument.cpp 57aba26
>
> Diff: http://git.reviewboard.kde.org/r/111560/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Friedrich W. H. Kossebau
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20130718/0718c2f7/attachment.htm>
More information about the calligra-devel
mailing list