D9824: Optimize inotify KDirWatch backend: map inotify wd to Entry

[René J.V. Bertin]

rjvbb added a comment.


  >   @rjvbb why did you request changes to proceed with this patch? The fact that there are more issues in KDirWatch does not mean we should hold up this approach.
  
  That should be obvious, no? You identified an issue but only addressed it in a single backend. As far as I can tell the same approach should work in the QFSW backend so I consider your current patch unfinished. At the very least you should present benchmark results that show the effect on the QFileSystemWatcher backend if they illustrate that this effect is in fact negligible.
  
  Note how I'm not asking things that require installing additional dependencies, write new unittests or even spend hours on it. The change looks trivial enough to refactor and apply to the QFSW backend.
  
  >   Also, please note how the fundamental issue described here affects more backends beside inotify, but they will need to be handled separately.
  
  I do NOT agree, not without proof that your simple fix does not work for at least the QFSW backend (which is probably the most commonly used across platforms).
  
  Your fix may not be the optimal one for all backends but shunning an improvement just because there might be a better one isn't very productive. Just leave a comment in the code or remark in the commit message if you have reason to believe that things can be improved even more.
  
  >   I added you to this review so that you can see how one could do it.
  
  And now just imagine reversing the roles. I would get exactly the same orders, probably requiring me to address the issue for all backends even if it required using different approaches.
  Not that I'm looking for payback, but I'm not going to drop my request just so no one might thinking I am in fact looking for that.
  
  This code has hardly evolved for years. Now someone with the required theoretical knowledge is looking into improving things. I vote that he tries to let the benefits of his improvement be reaped as widely as possible, while he is at it.

REPOSITORY
  R244 KCoreAddons

REVISION DETAIL
  https://phabricator.kde.org/D9824

To: mwolff, dfaure, rjvbb, #kdevelop
Cc: markg, #frameworks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180116/e067f034/attachment.html>