<table><tr><td style="">dfaure added inline comments.
</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/D26448">View Revision</a></tr></table><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/D26448#inline-172020">View Inline</a><span style="color: #4b4d51; font-weight: bold;">nicolasfella</span> wrote in <span style="color: #4b4d51; font-weight: bold;">krecentfilesmenu.cpp:150</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">I'm wondering how to best make this async without selling my soul to the mulitthreading devil. QtConcurrent::filtered looks interesting, but depending on QtConcurrent wouldn't be feasible, would it?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Depending on QtConcurrent is just fine. However QtConcurrent::filtered is for CPU-intensive operations, not for I/O operations. 1) you don't want to put blocking runnables in the global thread pool [can be solved by assigning to a different threadpool], 2) I don't think you really want to parallelize 16 calls to QFile::exists, for the case of a local physical harddisk? Not sure it would really harm (we're not reading 16 files, at least), but for sure the overhead of dispatching runnables to 16 threads would make it slower... One solution here might be just a dedicated thread iterating over the list.</p>

<p style="padding: 0; margin: 8px;">However: note that the usual KIO way to do file operations async isn't multithreading, it's rather KIO jobs.<br />
This would move the "5 minutes waiting for an NFS server" horror case into a kioslave rather than a thread, both achieve the desired outcome which is to not block the GUI thread.</p>

<p style="padding: 0; margin: 8px;">Keeping the order of the entries is going to be interesting. I guess we need a temporary data structure which stores "exists | does not exist" for each entry, and once all jobs are done, we go through that and fill the menu. Note that remote URLs should just be assumed to exist, we don't want to actually check (slow; might prompt for password; might error on different networks; etc.).</p>

<p style="padding: 0; margin: 8px;">Unlike Kai-Uwe, I think this should be a separate merge request though, it's all quite orthogonal to your work. As long as you confirm that filling the menu "later" has no negative impact on the API (i.e. the user of the class cannot assume that the menu is filled in immediately).</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R236 KWidgetsAddons</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D26448">https://phabricator.kde.org/D26448</a></div></div><br /><div><strong>To: </strong>nicolasfella, Frameworks, dfaure<br /><strong>Cc: </strong>broulik, elvisangelaccio, cfeck, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns<br /></div>