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

[Daniel Vrátil]

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

Ship it!


Ship It!

- Daniel Vrátil


On Nov. 26, 2014, 7:19 p.m., Milian Wolff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121247/
> -----------------------------------------------------------
> 
> (Updated Nov. 26, 2014, 7:19 p.m.)
> 
> 
> Review request for Akonadi, Daniel Vrátil and Volker Krause.
> 
> 
> Repository: akonadi
> 
> 
> Description
> -------
> 
> 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
> -----
> 
>   server/src/storage/querybuilder.h b380f93d8153ebf4581d24da9c820fd0b7b81701 
>   server/src/storage/querybuilder.cpp c07905906bdc712d41007ada753c2d821afd0a2d 
>   server/tests/unittest/querybuildertest.h 3bb6b22435816b17930956cfc0eb8541db279042 
>   server/tests/unittest/querybuildertest.cpp 0aba8a17ad7fb5c35f4b2b3e4366d11061b5329d 
> 
> 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/