D7742: KDevelop projects: optional use of KDirWatch and defer it and parser start to after project loading

[Milian Wolff]

mwolff requested changes to this revision.
mwolff added a comment.
This revision now requires changes to proceed.


  this all looks like a huge workaround - I think the time would be better spent in improving kdirwatch's performance internally, or coming up with a better way to watch a project. First of all, we need a clear way to reproduce the issue, i.e. a canonical "big" repository we can use for benchmarking. Then we need to use a proper profiler and look at the results there. Are the issues syscalls? Or something in userland that could be improved in Qt/KDE/KDevelop? Just disabling this feature sounds like the wrong approach to me.

INLINE COMMENTS

> abstractfilemanagerplugin.cpp:120
>      }
> -    delete m_watchers.take(project);
> +    if (ICore::self()->projectController()->watchAllProjectDirectories()) {
> +        if (m_watchers.contains(project)) {

if the setting has changed, you still may have a watcher from before which you want to delete here. I don't think you should handle the setting, but rather make the code fail gracefully if no watcher exists for the given project:

  delete m_watchers.value(project, nullptr);
  m_watchers.remove(project);

> abstractfilemanagerplugin.cpp:396
>      const QString path = folder->path().toLocalFile();
> -    m_watchers[folder->project()]->stopDirScan(path);
> +    if (ICore::self()->projectController()->watchAllProjectDirectories()) {
> +        if (m_watchers.contains(folder->project())) {

if the setting has changed, you still may have a watcher from before which you want to stop here, see above

> abstractfilemanagerplugin.cpp:410
>      const QString path = folder->path().toLocalFile();
> -    m_watchers[folder->project()]->restartDirScan(path);
> +    if (ICore::self()->projectController()->watchAllProjectDirectories()) {
> +        if (m_watchers.contains(folder->project())) {

dito, but slightly changed - if the setting changed, did you recreate the watchers?

> projectcontroller.cpp:91
>  
> +static QElapsedTimer m_timer;
> +

remove or move into the Private class below, don't make it static

> projectcontroller.cpp:916
>  
> -    reparseProject(project);
> +    // don't call reparseProject immediately, defer until all currently opening
> +    // projects have been imported. Parsing is done in the background but

this should be a separate patch, it has nothing to do with dirwatches

> projectcontroller.cpp:1288
> +{
> +    KDevelop::AbstractFileManagerPlugin *manager = 
> +        dynamic_cast<KDevelop::AbstractFileManagerPlugin*>(project->projectFileManager());

this looks wrong, add the API to IProject and call the virtual directly, obsoleting this whole method

REPOSITORY
  R32 KDevelop

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

To: rjvbb, #kdevelop, mwolff
Cc: mwolff, kossebau, arrowdodger, brauch, zhigalin, kdevelop-devel, geetamc, Pilzschaf, akshaydeo, surgenight
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170912/5a14ddfd/attachment-0001.html>