<table><tr><td style="">rjvbb marked 2 inline comments as done.<br />rjvbb 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/D8579" 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/D8579#163353" style="background-color: #e7e7e7;
          border-color: #e7e7e7;
          border-radius: 3px;
          padding: 0 4px;
          font-weight: bold;
          color: black;text-decoration: none;" rel="noreferrer">D8579#163353</a>, <a href="https://phabricator.kde.org/p/mwolff/" style="
              border-color: #f1f7ff;
              color: #19558d;
              background-color: #f1f7ff;
                border: 1px solid transparent;
                border-radius: 3px;
                font-weight: bold;
                padding: 0 4px;" rel="noreferrer">@mwolff</a> wrote:</div>
<div style="margin: 0;
          padding: 0;
          border: 0;
          color: rgb(107, 116, 140);"><p>thanks for the numbers and profile on the project filter, I guess I'll need to port it away from QRegExp to QRegularExpression, which will mean some custom parser to transfer unix style matching to regular expressions patterns. But it does seem to help (left is status quo, right is QRegularExpression with manual conversion of the default patters to regular expressions):</p></div>
</blockquote>

<p>...</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>Anyhow, not really related to this patch here, but good to keep in mind for the future</p></blockquote>

<p>Indeed. Are those gains still visible in real world usage - it doesn't look like filtering is a bottleneck operation like the dirwatcher population currently is? In a real world project import, a properly set up filter would exclude an in-tree build directory first, thereby skipping a whole bunch of files that would otherwise be excluded one by one. (IOW, it might already be very effective to optimise the order of the entries in the default filter.)</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/D8579#inline-37661" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">mwolff</span> wrote in <span style="color: #4b4d51; font-weight: bold;">abstractfilemanagerpluginimportbenchmark.cpp:67</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">why is this required?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">It replaces a full-blown ProjectController initialisation, and tells the ProjectFilterProvider to set up the project filter. Without that signal, the filter doesn't do its full job (it only filters out the .kdev4 directories).</p>

<p style="padding: 0; margin: 8px;">I tried setting up the ProjectController, but got crashes while plugins were being loaded that aren't of interest anyway, and in addition we'd have to import the test projects through the ProjectController in order for it to send out that signal. I don't think we want that here.</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/D8579#inline-37660" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">mwolff</span> wrote in <span style="color: #4b4d51; font-weight: bold;">abstractfilemanagerpluginimportbenchmark.cpp:131</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">this isn't needed anymore, is it? i.e. it should never happen?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I suppose it should indeed never happen. The call itself is required though (I checked again), and if it fails it will do so quietly and results will be for project import without filtering. I wouldn't mind putting an assert here, but a version that doesn't disappear in release builds.</p></div></div></div></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/D8579" rel="noreferrer">https://phabricator.kde.org/D8579</a></div></div><br /><div><strong>To: </strong>rjvbb, KDevelop<br /><strong>Cc: </strong>mwolff, kdevelop-devel, njensen, geetamc, Pilzschaf, akshaydeo, surgenight, arrowdodger<br /></div>