Bug 384866 - KDirWatch::addDir should pump the eventloop (call QCoreApplication::processEvents())
Summary: KDirWatch::addDir should pump the eventloop (call QCoreApplication::processEv...
Status: RESOLVED WORKSFORME
Alias: None
Product: frameworks-kcoreaddons
Classification: Frameworks and Libraries
Component: general (other bugs)
Version First Reported In: unspecified
Platform: Compiled Sources macOS
: NOR wishlist
Target Milestone: ---
Assignee: Michael Pyne
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-09-19 20:22 UTC by RJVB
Modified: 2020-10-11 04:33 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed/Implemented In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description RJVB 2017-09-19 20:22:11 UTC
Setting a dirwatcher on a large tree can be quite expensive on Mac. For example, a recursive KDirWatch::addDir() on the GCC source tree will take around 85 seconds on a 2.7Ghz i7 (with a fast HDD) while the KDirWatch dtor will take roughly twice that time. Linux timings on a much slower CPU are a fraction of that and negligible in probably most contexts where you'd want to create such a dirwatcher. 

I think this qualifies as the kind of lengthy operation for which Qt suggests to call QCoreApplication::processEvents() periodically. That would make it less invasive to call on the main thread, as applications (developed on Linux) might be wont to do.

The logical place for doing such a check would be in KDirWatchPrivate::addEntry(), just before the loop where it calls itself recursively. That can however cause a race condition: how to handle the situation where a user action leads to the destruction of the KDirWatch object itself (e.g. the user quits the application during the lengthy dirwatcher creation).
Comment 1 Harald Sitter 2020-09-06 21:10:07 UTC
Do it in a thread I'd suggest.
Comment 2 RJVB 2020-09-11 09:22:05 UTC
I don't think it's a good idea to handle GUI events anywhere but on the main thread - on Mac at least. Unless you meant to call KDirWatch::addDir() on its own thread?

This ticket is 2 years old. It's not impossible that I filed it before realising that Darwin, being a BSD variant, only has native support for watching directories for changes. I do remember that the 85s figure above comes from opening the GCC source tree in KDevelop, which will be default add every file and directory to its dirwatcher. I've worked around the issue by overhauling KDevelop's monitoring implementation to watch only directories (in a local patch but by now one more or less of those doesn't make a lot of difference anymore...)
Comment 3 Harald Sitter 2020-09-11 10:28:50 UTC
Use KDirWatch in a thread is what I mean.
Sending synchronization between a file watcher thread and the gui thread is loads less problematic then effectively doing nested ::exec() behavior inside a library. Nested execs are breaking client code all the time. In particular with QML. And since we have no control over the client code it really is not safe to process events inside a blocking function.
Alternatively of course simply breaking up the addDir() calls on the gui thread does help as well, in the end the API allows for much finer control over how many watches are being created per addDir call via the WatchModes argument.
Comment 4 Bug Janitor Service 2020-09-26 04:33:10 UTC
Dear Bug Submitter,

This bug has been in NEEDSINFO status with no change for at least
15 days. Please provide the requested information as soon as
possible and set the bug status as REPORTED. Due to regular bug
tracker maintenance, if the bug is still in NEEDSINFO status with
no change in 30 days the bug will be closed as RESOLVED > WORKSFORME
due to lack of needed information.

For more information about our bug triaging procedures please read the
wiki located here:
https://community.kde.org/Guidelines_and_HOWTOs/Bug_triaging

If you have already provided the requested information, please
mark the bug as REPORTED so that the KDE team knows that the bug is
ready to be confirmed.

Thank you for helping us make KDE software even better for everyone!
Comment 5 Bug Janitor Service 2020-10-11 04:33:12 UTC
This bug has been in NEEDSINFO status with no change for at least
30 days. The bug is now closed as RESOLVED > WORKSFORME
due to lack of needed information.

For more information about our bug triaging procedures please read the
wiki located here:
https://community.kde.org/Guidelines_and_HOWTOs/Bug_triaging

Thank you for helping us make KDE software even better for everyone!