[Kmymoney-devel] Review Request 120815: Use implicit sharing in AlkValue

Cristian Oneț onet.cristian at gmail.com
Fri Nov 7 09:02:10 UTC 2014



> On Nov. 5, 2014, 6:51 p.m., Thomas Baumgart wrote:
> > libalkimia/alkvalue.cpp, line 86
> > <https://git.reviewboard.kde.org/r/120815/diff/2/?file=326266#file326266line86>
> >
> >     Does this get initialized under all circumstances?
> 
> Cristian Oneț wrote:
>     It's a static so it's initialized once. The purpose of this value is to make constructing/copying zero values fast. Do you see something wrong with this?
> 
> Christian David wrote:
>     If someone uses a static or global AlkValue() it is not guaranteed that the static ```AlkValue::sharedZero``` was initialized before. I have a suggestion how to solve that in /r/121030.
> 
> Thomas Baumgart wrote:
>     Yes, please take a look at #121030. These two patches combined (I don't know why Christian did not simply comment here but opened a new review) we should be able to ship it.

OK, you are right I didn't thought about that, nice catch! :) I will merge [121030](https://git.reviewboard.kde.org/r/121030/) with this request.


- Cristian


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


On Nov. 5, 2014, 6:21 p.m., Cristian Oneț wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120815/
> -----------------------------------------------------------
> 
> (Updated Nov. 5, 2014, 6:21 p.m.)
> 
> 
> Review request for KMymoney and Thomas Baumgart.
> 
> 
> Repository: alkimia
> 
> 
> Description
> -------
> 
> Before this the biggest cost of using an AlkValue object, implicitly a KMyMoneyMoney object was assignment, construction and destruction.
> 
> By using implicit sharing combined with a shared zero value, copying on assignment and construction can be greatly reduced.
> 
> Implicit sharing was easy to implement using QSharedDataPointer.
> 
> Bumped the library version because API changes were necessary. The mpq_class &valueRef() const method was bad beacause it was returning a non-const reference from a const function. Now There is a const version which will not detach the shared data while the non-const version will detach from the shared data.
> 
> 
> Diffs
> -----
> 
>   libalkimia/CMakeLists.txt 3dbe4db 
>   libalkimia/alkvalue.h 7c02403 
>   libalkimia/alkvalue.cpp ae5d3a4 
> 
> Diff: https://git.reviewboard.kde.org/r/120815/diff/
> 
> 
> Testing
> -------
> 
> Succesfully ran KMyMoney and KMyMoney tests. See the attached screeshots for the callgrind data about AlkValue obtained while loading the same KMyMoney file without and with this change. Note that the second run also contains some MyMoneyMoney optimizations based on this feature (subject of a different review request).
> 
> 
> File Attachments
> ----------------
> 
> Callgrind data using AlkValue from alkimia 4.3.2
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/10/26/f79cfa4b-b01a-4d14-adbe-dc905ad1a4c9__alk-value.png
> Callgrind data using AlkValue from alkimia 4.4.0 (with implicit sharing)
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/10/26/ceb1cb24-fe5e-414e-a0c1-09746bcb9bcc__alk-value-implicitly-shared.png
> Callgrind data while loading file by KMyMoney using review 120818 and alkimia 4.3.2
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/10/26/9d142707-962b-434a-b847-8af7154ff4a1__alk-value-4.3.2.png
> Callgrind data while loading register by KMyMoney using review 120818 and alkimia 4.3.2
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/10/26/25200104-be0e-434e-941d-c7c51c9bcb2c__register-load-4.3.2.png
> Callgrind data while loading register by KMyMoney using review 120818 and alkimia 4.4.0
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/10/26/58ee7a28-2723-4b2c-9818-e82b0b6a03e8__register-load-4.4.0.png
> Callgrind data while editing transaction by KMyMoney using review 120818 and alkimia 4.3.2
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/10/26/6f25d85b-3c3e-4d92-9f1a-1315daf3788c__transaction-edit-4.3.2.png
> Callgrind data while editing transaction by KMyMoney using review 120818 and alkimia 4.4.0
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/10/26/e4d5d50f-efb8-4f0b-bed8-23fa1c5db391__transaction-edit-4.4.0.png
> Callgrind data while loading file by KMyMoney using review 120818 and alkimia 4.4.0
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/10/26/8808ebc3-a112-4425-b35e-05acd8ba94d1__alk-value-4.4.0.png
> 
> 
> Thanks,
> 
> Cristian Oneț
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kmymoney-devel/attachments/20141107/aa47a4e2/attachment-0001.html>


More information about the KMyMoney-devel mailing list