<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/120815/">https://git.reviewboard.kde.org/r/120815/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On Noiembrie 5th, 2014, 6:51 p.m. UTC, <b>Thomas Baumgart</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/120815/diff/2/?file=326266#file326266line86" style="color: black; font-weight: bold; text-decoration: underline;">libalkimia/alkvalue.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">AlkValue::AlkValue() :</pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">81</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">AlkValue</span><span class="o">::</span><span class="n">AlkValue</span><span class="p">()</span> <span class="o">:</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">86</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">QSharedDataPointer</span><span class="o"><</span><span class="n">AlkValue</span><span class="o">::</span><span class="n">Private</span><span class="o">></span> <span class="n">AlkValue</span><span class="o">::</span><span class="n">sharedZero</span><span class="p">(</span><span class="k">new</span> <span class="n">AlkValue</span><span class="o">::</span><span class="n">Private</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Does this get initialized under all circumstances?</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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?</p></pre>
<br />
<p>- Cristian</p>
<br />
<p>On Noiembrie 5th, 2014, 6:21 p.m. UTC, Cristian Oneț wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KMymoney and Thomas Baumgart.</div>
<div>By Cristian Oneț.</div>
<p style="color: grey;"><i>Updated Noie. 5, 2014, 6:21 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
alkimia
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Before this the biggest cost of using an AlkValue object, implicitly a KMyMoneyMoney object was assignment, construction and destruction.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">By using implicit sharing combined with a shared zero value, copying on assignment and construction can be greatly reduced.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Implicit sharing was easy to implement using QSharedDataPointer.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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).</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>libalkimia/CMakeLists.txt <span style="color: grey">(3dbe4db)</span></li>
<li>libalkimia/alkvalue.h <span style="color: grey">(7c02403)</span></li>
<li>libalkimia/alkvalue.cpp <span style="color: grey">(ae5d3a4)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/120815/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<li><a href="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.3.2</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/10/26/ceb1cb24-fe5e-414e-a0c1-09746bcb9bcc__alk-value-implicitly-shared.png">Callgrind data using AlkValue from alkimia 4.4.0 (with implicit sharing)</a></li>
<li><a href="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 file by KMyMoney using review 120818 and alkimia 4.3.2</a></li>
<li><a href="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.3.2</a></li>
<li><a href="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 loading register by KMyMoney using review 120818 and alkimia 4.4.0</a></li>
<li><a href="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.3.2</a></li>
<li><a href="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 editing transaction by KMyMoney using review 120818 and alkimia 4.4.0</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/10/26/8808ebc3-a112-4425-b35e-05acd8ba94d1__alk-value-4.4.0.png">Callgrind data while loading file by KMyMoney using review 120818 and alkimia 4.4.0</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>