<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
    <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="https://git.reviewboard.kde.org/r/118107/">https://git.reviewboard.kde.org/r/118107/</a>
     </td>
    </tr>
   </table>
   <br />





<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On May 12th, 2014, 11:08 p.m. UTC, <b>Marko Käning</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I'll incorporate it as soon as possible in a updated version of https://git.reviewboard.kde.org/r/112885/ .

Thanks, Alvaro, for doing this job.

I'll also report back as soon as I can regarding speed.</pre>
 </blockquote>




 <p>On May 12th, 2014, 11:13 p.m. UTC, <b>Marko Käning</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Only now I realise that you did not cache the value in any case which lets you re-iterate through all transactions at every call...

Wasn't there some caching built-in for the corresponding MyMoneyFile::balance()?</pre>
 </blockquote>





 <p>On May 13th, 2014, 5:59 a.m. UTC, <b>Marko Käning</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Alvaro, I successfully incorporated your patch using git master and ran my actual kmy with it.

My account with the most transactions didn't show any significant change of speed (although there's seemingly no caching up to now - but perhaps I am mistaken there?).

This was measured manually as the time lag until the corresponding ledger appears after clicking the ledger icon - i.e. not applying any debug output - which resulted in a duration of about 1.5s.</pre>
 </blockquote>





 <p>On May 13th, 2014, 11:38 a.m. UTC, <b>Alvaro Soliverez</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">TBH, at first I thought this feature was only used during reconciliation and not every time the ledger is loaded. 
In any case, there are still some optimizations to be made (eg. caching, calculating the cleared balance with a different method if the account was never reconciled, etc.), but it only makes sense to optimize once a problem has been detected.</pre>
 </blockquote>





 <p>On May 13th, 2014, 3:12 p.m. UTC, <b>Christian David</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">"it only makes sense to optimize once a problem has been detected" +1

I tried this patch with a huge xml test file (about 100 000 transactions and 50 000 per account). By hand I could not find a difference in speed. This makes sense, as the main problem is probably loading and copying the data to and within main memory. Compared to that just reading it to calculate a sum should be negligible.</pre>
 </blockquote>





 <p>On May 13th, 2014, 8:18 p.m. UTC, <b>Marko Käning</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">You are right. Let's ship it.</pre>
 </blockquote>








</blockquote>

<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">There's also one fundamental difference with the balance() method. That one needs a cache because the AccountsModel calls it every time a transaction is updated for all accounts. This is only needed for one particular account when the ledger is displayed.</pre>
<br />










<p>- Alvaro</p>


<br />
<p>On May 12th, 2014, 10:53 p.m. UTC, Alvaro Soliverez wrote:</p>








<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
 <tr>
  <td>

<div>Review request for KMymoney.</div>
<div>By Alvaro Soliverez.</div>


<p style="color: grey;"><i>Updated May 12, 2014, 10:53 p.m.</i></p>









<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
kmymoney
</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;">It calculates cleared balance for an account in MyMoneyFile, instead of doing it in the ledger view (kgloballedgerview.cpp)

I'm posting it here because I'm a bit concerned on the performance it may have for everyday use.
I'm keen on optimizing, but this kind of code should be removed from the views in the long-term.</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;">I tested with a small file. It works ok. I still have to add a unit test for it.</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>kmymoney/mymoney/mymoneyfile.h <span style="color: grey">(3a39ba0)</span></li>

 <li>kmymoney/mymoney/mymoneyfile.cpp <span style="color: grey">(4f6d277)</span></li>

 <li>kmymoney/views/kgloballedgerview.cpp <span style="color: grey">(d2d626c)</span></li>

</ul>

<p><a href="https://git.reviewboard.kde.org/r/118107/diff/" style="margin-left: 3em;">View Diff</a></p>







  </td>
 </tr>
</table>








  </div>
 </body>
</html>