<table><tr><td style="">mwolff added a comment.
</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/D7995" rel="noreferrer">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D7995#168405" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;" rel="noreferrer">D7995#168405</a>, <a href="https://phabricator.kde.org/p/rjvbb/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;" rel="noreferrer">@rjvbb</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><p>Milian Wolff wrote on 20171116::10:55:31 re: "<a href="https://phabricator.kde.org/D7995" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;" rel="noreferrer">D7995</a>: KDevelop: address dirwatching inefficiency (WIP/PoC)"</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">I **still** think the number of watched directories is completely useless information. The total time is interesting, and whether the unit tests still work that ensure the correct dirs are watched. This means: remove the project watcher class, keep using dirwatcher directly.</pre></div></blockquote>
<p>We still disagree on that subject then and I can still keep the benchmark crippled if that's what you want. Did you actually read the argument why I reintroduced this output, how total time is now dependent on a number of watched directories that is no longer an external given but is now essentially an *un*controlled variable.</p>
<p>Consider 2 directories with exactly the same number of files and directories. One has a substantial subtree under a folder that will be filtered out, the other not. Which one do you think will import faster, and do you think you'll understand that difference at a glance from benchmark output that doesn't includes total time but not the number of timed operations?</p></div>
</blockquote>
<p>The benchmark changes its performance based on the commit which hopefully improves the performance. The commit message also includes the numbers and shows the advantage of the new behavior.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>Ultimately I don't care because I don't write that benchmark for myself - but removing this output will be done under protest and I'll be obliged to leave some trace of that in a comment.</p></blockquote>
<p>I won't accept it with traces of useless stuff in it.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>Now, the ProjectWatcher class. This one doesn't only keep count of the number of watched directories. It also prevents directories from being added multiple times which leads to unclear behaviour inside KDirWatch and may ultimately lead to directories that aren't unwatched when necessary. So it'll have to stay.</p></blockquote>
<p>Unclear behavior? That needs to be fixed in KDirWatch upstream then.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>please document why queuing is required here?</p></blockquote>
<p>Will do, but that'll just say it's following good-practice instructions from Qt devs. Qt is indeed supposed be able to detect when queued connections are required but it cannot do so 100% reliably and this is a case where I've seen it go wrong</p></blockquote>
<p>FUD. I'm a Qt dev, and I told you how it works which should be reliable for your purpose here.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>(You may not remember, but I also tested making the other connections queued, which shouldn't a prior have any impact AFAIU. It did: caused lock-ups.)</p></blockquote>
<p>If you make stuff queued it does change everything - i.e. direct signal emission will never happen, even when the threads are the same.</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R32 KDevelop</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D7995" rel="noreferrer">https://phabricator.kde.org/D7995</a></div></div><br /><div><strong>To: </strong>rjvbb, KDevelop, mwolff<br /><strong>Cc: </strong>aaronpuchert, arrowdodger, kfunk, dfaure, mwolff, brauch, kdevelop-devel, njensen, geetamc, Pilzschaf, akshaydeo, surgenight<br /></div>