[Kde-pim] Review Request 121247: Test whether compiler supports all required C++11 features at configure time

Milian Wolff mail at milianw.de
Wed Nov 26 18:16:43 GMT 2014


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

(Updated Nov. 26, 2014, 6:16 p.m.)


Review request for Akonadi, Daniel Vrátil and Volker Krause.


Changes
-------

style fixes and removed the QHash, just a plain array with two entries is enough.


Summary (updated)
-----------------

Test whether compiler supports all required C++11 features at configure time


Repository: akonadi


Description (updated)
-------

To prevent ugly compilation errors when someone tries to compile Akonadi
with a compiler that does not support all C++11 features we use, we run
a try_compile check in CMakeLists.txt.

prevent starting a QTimer with a negative interval Review: 120800


Convert some qDebugs to akDebugs

This should make Akonadi in release mode even less chatty.

Optimize: Reduce the amount of allocations required to build a query.

The initial implementation of the QueryBuilder was quite naive, when
you look at the amount of string allocations it does to build the
final query we sent to the SQL server.

This was found with Linux perf (no, not even heaptrack!). It
showed a huge number of cycles spent in malloc/free, all called
eventually by the QueryBuilder.

This patch removes most of these allocations. It can further be
improved in the future, I bet. Also, the amount of queries we create
is pretty large. I guess using stored procedures or something similar
might also help the performance. At least, we should try to "remember"
some of our queries, and make it possible to reuse them in the
functions that run often.

The added benchmark shows that the cost is not as big as I'd initially
assumed. There are simply many more allocation occurrences in Akonadi
currently. Still, I think it's worth it, as it also decreases the
memory fragmentation and improves cache locality:

Before:
RESULT : QueryBuilderTest::benchQueryBuilder():
     0.0115 msecs per iteration (total: 116, iterations: 10000)

113.10MB bytes allocated in total (ignoring deallocations)
over 1203089 calls to allocation functions.
peak heap memory consumption: 254.46KB

After:
RESULT : QueryBuilderTest::benchQueryBuilder():
     0.0065 msecs per iteration (total: 66, iterations: 10000)

62.42MB bytes allocated in total (ignoring deallocations)
over 343089 calls to allocation functions.
peak heap memory consumption: 254.96KB

So before, we had approx. 60 allocations per query build in the
benchmark (note that Qt for some reason executes the loop twice,
so while the time is measured for 10k iterations, heaptrack will
see 20k). With this patch applied, we only need ~20 allocations
per query we build up.

The remaining allocations are the various append operations to
the QList/QVectors mostly, as well as QueryBuilder::addAggregation.

REVIEW: 121247


Diffs (updated)
-----

  server/src/storage/querybuilder.cpp c07905906bdc712d41007ada753c2d821afd0a2d 
  server/tests/unittest/querybuildertest.h 3bb6b22435816b17930956cfc0eb8541db279042 
  server/tests/unittest/querybuildertest.cpp 0aba8a17ad7fb5c35f4b2b3e4366d11061b5329d 
  CMakeLists.txt e081d23c1f9871823cea157fcd9a5499dd477130 
  server/src/collectionscheduler.cpp 8d4cd5c92ce32195c80b2d3092c12e857599220b 
  server/src/handler/merge.cpp fffe10099fb7f983c34e887bec8ea8312cc68b6f 
  server/src/handler/modify.cpp 9671fb94630e81aa3da28251e9beb6c61e73b8fb 
  server/src/handler/remove.cpp 090531f961e266b48a3a6487b838e0c96d44acba 
  server/src/search/searchmanager.cpp 35e76e1881575553da493dd23d3fd1a0b618bbb1 
  server/src/storage/querybuilder.h b380f93d8153ebf4581d24da9c820fd0b7b81701 

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


Testing
-------

querybuilder test runs, KMail runs - no errors shown on the CLI.


Thanks,

Milian Wolff

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/


More information about the kde-pim mailing list