[Kde-pim] Review Request 121247: Optimize: Reduce the amount of allocations required to build a query.

[Milian Wolff]

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

(Updated Nov. 26, 2014, 11:31 a.m.)


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


Changes
-------

added a benchmark and some hard numbers.


Repository: akonadi


Description (updated)
-------

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.0074 msecs per iteration (total: 75, iterations: 10000)

68.50MB bytes allocated in total (ignoring deallocations)
over 423091 calls to allocation functions.
peak heap memory consumption: 255.26KB

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/tests/unittest/querybuildertest.cpp 0aba8a17ad7fb5c35f4b2b3e4366d11061b5329d 
  server/src/storage/querybuilder.h b380f93d8153ebf4581d24da9c820fd0b7b81701 
  server/src/storage/querybuilder.cpp c07905906bdc712d41007ada753c2d821afd0a2d 
  server/tests/unittest/querybuildertest.h 3bb6b22435816b17930956cfc0eb8541db279042 

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/