<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/114349/">http://git.reviewboard.kde.org/r/114349/</a>
</td>
</tr>
</table>
<br />
<div>
<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/114349/diff/2/?file=223283#file223283line161" style="color: black; font-weight: bold; text-decoration: underline;">src/plugins/akonadi/akonadidatasource.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; ">private Q_SLOTS:</pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">public Q_SLOTS:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">144</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">AkonadiContact</span><span class="p">(</span><span class="k">const</span> <span class="n">QString</span> <span class="o">&</span><span class="n">contactId</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">132</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">AkonadiContact</span><span class="p">(</span><span class="k">const</span> <span class="n">QString</span> <span class="o">&</span><span class="n">contactId</span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="n"><span class="hl">QAbstractItemModel</span></span><span class="hl"> </span><span class="o"><span class="hl">*</span></span><span class="n"><span class="hl">model</span></span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This defeats the entire point of this API.
The API was designed so that if we only want to fetch data for one contact (which is a valid use case a lot - like in the text-ui or konversation or kmail etc.) we don't load /every/ contact from every data source nor do we want to monitor every contact.
This was all cleverly designed to minimise traffic as much as possible and this change would ruin all of that.
(also this class just reproduces DefaultContactMonitor)
I guess it's my fault for not documenting yet.</pre>
</div>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">On the ETM model change, I struggle to believe that Akonadi fetching the data then building a model only for us to then scrape the model for the data is going to be more efficient than us just loading the data.</pre>
<p>- David Edmundson</p>
<br />
<p>On December 8th, 2013, 2:48 a.m. UTC, Dan Vrátil wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://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 Telepathy.</div>
<div>By Dan Vrátil.</div>
<p style="color: grey;"><i>Updated Dec. 8, 2013, 2:48 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
libkpeople
</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;">Use a single ETM instead of manually fetching all contacts and then installing a new Akonadi Monitor for each single AkonadiContact.
ETM (with EntityMimeTypeFilterModel on top) will do the CollectionFetchJobs and ItemFetchJobs and data handling for us, which only leaves us with handling of changes, which we do in slots for QAbstractItemModel's rowsInserted(), rowsRemoved() and dataChanged() signals. This is much better than having many Akonadi monitors, because for each change notification that Akonadi server generates, it has to iterate over all registered monitors and compare the notification against each monitor's filter. With lots of monitors, this can impact performance of the server. Although the AkonadiContacts with their Akonadi monitors are created on demand (so there's none at the beginning), they are not removed (probably cached somewhere in PersonsModel), so the number of active Akonadi monitors tends to grow - for instance going through all contacts in PersonViewer will leave you with one monitor for each contact fed from Akonadi.
>From Akonadi POV, having one properly configured monitor (hidden in ETM) and working with data via ETM is a better approach.</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;">Going through all contacts in PersonViewer includes all contacts from Akonadi, and the application uses only one Akonadi Monitor all the time.</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>CMakeLists.txt <span style="color: grey">(889e96d)</span></li>
<li>src/plugins/akonadi/akonadidatasource.h <span style="color: grey">(b09edf8)</span></li>
<li>src/plugins/akonadi/akonadidatasource.cpp <span style="color: grey">(193de78)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/114349/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>