<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="http://git.reviewboard.kde.org/r/101046/">http://git.reviewboard.kde.org/r/101046/</a>
     </td>
    </tr>
   </table>
   <br />








<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On April 7th, 2011, 2:43 p.m., <b>Aaron J. Seigo</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="http://git.reviewboard.kde.org/r/101046/diff/1/?file=13903#file13903line376" style="color: black; font-weight: bold; text-decoration: underline;">applets/comic/comic.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 1)

    </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; ">void ComicApplet::dataUpdated( const QString&amp;, const Plasma::DataEngine::Data &amp;data )</pre></td>
   <td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void ComicApplet::dataUpdated( const QString &amp;source, const Plasma::DataEngine::Data &amp;data )</pre></td>

  </tr>
 </tbody>




 
 



 <tbody>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
    <th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">376</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="n">mEngine</span><span class="o">-&gt;</span><span class="n">disconnectSource</span><span class="p">(</span><span class="n">source</span><span class="p">,</span> <span class="k">this</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;">why only on non-error?

also, this should happen after accessing the values of data as it could (in theory, strictly by the API docs) result in data becoming unset.</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;">The initial idea was to send the data if there is no error anymore. E.g. there is no internet connection, thus the data engine reports an error. Now if there happens to be an internet connection the data engine automatically trys again and then sets the data. As the applet is still connected it would then get the comic.

The disadvantage of this approach is that I do not specify which type of error happened in the datasource. Thus it might very well be that it will be never taken care of.</pre>
<br />

<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On April 7th, 2011, 2:43 p.m., <b>Aaron J. Seigo</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="http://git.reviewboard.kde.org/r/101046/diff/1/?file=13903#file13903line463" style="color: black; font-weight: bold; text-decoration: underline;">applets/comic/comic.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 1)

    </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; ">void ComicApplet::dataUpdated( const QString&amp;, const Plasma::DataEngine::Data &amp;data )</pre></td>
   <td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void ComicApplet::dataUpdated( const QString &amp;source, const Plasma::DataEngine::Data &amp;data )</pre></td>

  </tr>
 </tbody>



 
 




 <tbody>

  <tr>
    <th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">460</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="n">mEngine</span><span class="o">-&gt;</span><span class="n">query</span><span class="p">(</span> <span class="n">mComicIdentifier</span> <span class="o">+</span> <span class="sc">&#39;:&#39;</span> <span class="o">+</span> <span class="n">mNextIdentifierSuffix</span> <span class="p">);</span></pre></td>
    <th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">463</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">mNextIdentifierSuffix</span><span class="p">.</span><span class="n">isEmpty</span><span class="p">())</span> <span class="p">{</span></pre></td>
  </tr>

 </tbody>



 
 



 <tbody>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
    <th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">464</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">            <span class="n">m_prefetcher</span><span class="o">-&gt;</span><span class="n">prefetch</span><span class="p">(</span><span class="n">mComicIdentifier</span> <span class="o">+</span> <span class="sc">&#39;:&#39;</span> <span class="o">+</span> <span class="n">mNextIdentifierSuffix</span><span class="p">);</span></pre></td>
  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
    <th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">465</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="p">}</span></pre></td>
  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
    <th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">466</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">mPreviousIdentifierSuffix</span><span class="p">.</span><span class="n">isEmpty</span><span class="p">())</span> <span class="p">{</span></pre></td>
  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
    <th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">467</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">            <span class="n">m_prefetcher</span><span class="o">-&gt;</span><span class="n">prefetch</span><span class="p">(</span><span class="n">mComicIdentifier</span> <span class="o">+</span> <span class="sc">&#39;:&#39;</span> <span class="o">+</span> <span class="n">mPreviousIdentifierSuffix</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;">this should be doable without a separate QObject? simply connect the source to the ComicApplet as it was, then in dataUpdated do:

if ( source != mOldSource ) {
    disconnectSource( source );
    return;
}

that should have the same affect, looking at the code?</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;">Yes. I thought it to be nicer that way but as you said it is not needed.</pre>
<br />




<p>- Matthias</p>


<br />
<p>On April 7th, 2011, 2:08 p.m., Matthias Fuchs wrote:</p>






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

<div>Review request for Plasma and Aaron J. Seigo.</div>
<div>By Matthias Fuchs.</div>


<p style="color: grey;"><i>Updated April 7, 2011, 2:08 p.m.</i></p>




<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;">The comic plasmoid uses prefetching by calling Plasma::DataEngine::query(source).
Yet apparantly these created sources do not get removed automatically (?) causing an increase of memory usage the more comics are viewed.

E.g. looking at 15 strips of Lackadaisy that had not been cached before caused a memory usage of 114 MB for the plasmoidviewer. The patch reduces the usage in the same case to 52.9 MB. Each of these comics has a size between 1.2 MB and 2.6 MB.
So please comment on the patch! :)

I wonder why it is still that high with the patch after looking at some comics, so if you can help to reduce that further that would be great.
Further I wonder if some of the perceived &quot;plasma mem leak&quot; reports are caused by the way the comic plasmoid works.</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>applets/comic/CMakeLists.txt <span style="color: grey">(8513d9c)</span></li>

 <li>applets/comic/comic.h <span style="color: grey">(7134c72)</span></li>

 <li>applets/comic/comic.cpp <span style="color: grey">(3785f4f)</span></li>

 <li>applets/comic/prefetcher.h <span style="color: grey">(PRE-CREATION)</span></li>

 <li>applets/comic/prefetcher.cpp <span style="color: grey">(PRE-CREATION)</span></li>

</ul>

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




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








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