D7995: KDevelop: address dirwatching inefficiency (WIP/PoC)

René J.V. Bertin noreply at phabricator.kde.org
Tue Sep 26 21:35:12 UTC 2017


rjvbb marked 4 inline comments as done.
rjvbb added a comment.


  I'll see if I can use the abstractfilemanagerpluginimporttest to write a real-world benchmark. A priori it shouldn't be necessary to write a low-level, synthetic benchmark given the huge differences one can observe with simple timers of real-world situations.
  I have already been spending way more time on this than I should.

INLINE COMMENTS

> mwolff wrote in iprojectcontroller.h:123
> remove, I don't see the need for this. all project dirs should always be watched

I did explain why I had left it in...

> mwolff wrote in abstractfilemanagerplugin.cpp:126
>   if (auto* watcher = m_watchers.take(project)) {
>       delete watcher;
>   }

Dropped the size indication and went back to an earlier, simpler suggestion.

> mwolff wrote in abstractfilemanagerplugin.cpp:288
> why was this removed?

As far as I understand it is no longer necessary because we no longer get the `created` signal. When watching a folder for change you get `dirty` signals that mean only that something in the directory has changed, and that's handled in the "force reload of" case above (`eventuallyReadFolder()` does a recursive read, right?).

Do you remember exactly what cases were handled by this code?

> mwolff wrote in abstractfilemanagerplugin.cpp:477
> why don't we listen to created anymore?

Because when watching directories you only get that signal when a previously watched, deleted directory is recreated. We'll always get a dirty signal for such an event too (I checked).

> mwolff wrote in filemanagerlistjob.cpp:42
> allocates memory for no apparent gain, remove? or just use the project name, that and the class name is enough debug info

it doesn't even have the intended effect (I hoped to be able identify the thread by name). I guess I forgot to remove it.

> mwolff wrote in filemanagerlistjob.h:44
> why is the watcher optional?

It no longer has to be, now that I've removed the possibility to turn off dirwatching which I had left in for testing.

REPOSITORY
  R32 KDevelop

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

To: rjvbb, #kdevelop, mwolff
Cc: dfaure, mwolff, brauch, kdevelop-devel, geetamc, Pilzschaf, akshaydeo, surgenight, arrowdodger
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170926/82ac16da/attachment.html>


More information about the KDevelop-devel mailing list