<table><tr><td style="">dvratil requested changes to this revision.<br />dvratil added a comment.<br />This revision now requires changes to proceed.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D29602">View Revision</a></tr></table><br /><div><div><p><tt style="background: #ebebeb; font-size: 13px;">collectionForId</tt> is a good name, I think.</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D29602#inline-169943">View Inline</a><span style="color: #4b4d51; font-weight: bold;">storagemodel.cpp:263</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span class="p">}</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span class="n">mi</span><span style="color: #aa2211">-></span><span class="n">setFolder</span><span class="p">(</span><span class="n">folder</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">This will use a ton of memory and will make populating the model slow - for each single email you will need to resolve the real parent collection, traverse the collection parent chain and assemble the string.</p>

<p style="padding: 0; margin: 8px;">I suggest you add a <tt style="background: #ebebeb; font-size: 13px;">QHash<Collection::Id, QString></tt> lookup table into the StorageModel and try to look up the path by the collection ID there first, then fallback to building the path and inserting it into the hash map. This way we will make use of the implicit QString sharing so all items from the same collection will share the same single string with the path reducing memory footprint, and  we will avoid assembling the string every time, which is more costly than a hash map lookup.</p>

<p style="padding: 0; margin: 8px;">Remember to clear the hash map when the selected folder is changed.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D29602#inline-169934">View Inline</a><span style="color: #4b4d51; font-weight: bold;">storagemodel.cpp:506</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span style="color: #74777d">// get index in EntityTreeModel</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span style="color: #aa4000">const</span> <span class="n">QModelIndex</span> <span class="n">idx</span> <span style="color: #aa2211">=</span> <span class="n">EntityTreeModel</span><span style="color: #aa2211">::</span><span class="n">modelIndexForCollection</span><span class="p">(</span><span class="n">etm</span><span class="p">,</span> <span class="n">Collection</span><span class="p">(</span><span class="n">colId</span><span class="p">));</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span class="n">Q_ASSERT</span><span class="p">(</span><span class="n">idx</span><span class="p">.</span><span class="n">isValid</span><span class="p">());</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">You can pass the <tt style="background: #ebebeb; font-size: 13px;">d->mChildrenFilterModel</tt> here instead of the <tt style="background: #ebebeb; font-size: 13px;">etm</tt>, the code inside is clever enough to find the ETM source model.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R94 PIM: Message Library</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D29602">https://phabricator.kde.org/D29602</a></div></div><br /><div><strong>To: </strong>gordin, VDG, dvratil<br /><strong>Cc: </strong>dvratil, kde-pim, jamesth, fbampaloukas, dvasin, hrouis, rodsevich, ach, winterz, vkrause, mlaurent, knauss<br /></div>