Opened 13 years ago

Closed 13 years ago

#231 closed defect (fixed)

Problem creating multiple QFilesystemWatchers

Reported by: rudi Owned by:
Priority: major Milestone: Qt 4.7
Component: QtCore Version:
Severity: low Keywords:
Cc:

Description

When QT Creator starts up, the following message appears:

QOS2FileSystemWatcherEngine:: DosOpen(\PIPE\XNOTIFY) returned 231

Attachments (1)

qfilesystemwatcher_os2.diff (2.7 KB ) - added by rudi 13 years ago.
Possible fix

Download all attachments as: .zip

Change History (11)

by rudi, 13 years ago

Attachment: qfilesystemwatcher_os2.diff added

Possible fix

comment:1 by rudi, 13 years ago

Component: GeneralQtCore
Milestone: Qt EnhancedQt 4.7
Version: 4.6.3

It might be considered to use only a single instance of QOS2FileSystemWatcherEngine that is shared between all instances of QFileSystemWatcher.

comment:2 by Dmitry A. Kuminov, 13 years ago

I will look at it now.

But anyway, we will disable the native watcher due to the fact it does not properly notify us about file stamp or size changes. See ticket:205#comment:25.

BTW, I also checked that WPS behaves just like that too: no updates in the folder when the file stamp/size changes. Which proves the bug is in the Dos*ChangeNotify implementation, so that we can't fix it. Silly.

comment:3 by Dmitry A. Kuminov, 13 years ago

I checked, Qt Creator uses a lot of QFileSystemWatcher instances indeed. While I have never seen errors like ERROR_PIPE_BUSY myself, it's actually possible that the client (Qt) attempts to connect before the server (XWP) enters the next connection wait state with DosConnectNPipe(). Timings talks.

Though the proper solution for situations like that is to use DosWaitNPipe() (instead of DosSleep()ing and retrying). So I will go that way.

I will also make a single instance of QOS2FileSystemWatcherEngine shared between all QFileSystemWatcher ones -- this really makes sense since each QOSFileSysmteWatcher receives notifications for *all* file changes anyway.

comment:4 by Dmitry A. Kuminov, 13 years ago

The first part is done in r942.

comment:5 by Dmitry A. Kuminov, 13 years ago

The second part is done in r944.

comment:6 by Dmitry A. Kuminov, 13 years ago

I actually came up with quite an elegant solution for the mentioned lack of file date/size/attributes change notification in the OS/2 Dos*ChangeNotify API. I introduced one more thread for that which only checks for file info changes using the DosQueryPathInfo() call directly (which is relatively fast compared to QFileInfo(), stat() and friends) and nothing more. The rest (file renaming/addition/deletion) is still detected by the Dos*ChangeNotify API.

It turned out that in practice, applications don't watch for too many individual files and directories. For example, QFileDialog only watches a single directory at a time (the one being currently shown), Qt Creator watches all files open in the editor + all .pro files in the project (so usuall 10-20 in total or so).

This gives us quite acceptable performance and barely noticeable CPU load increase when this second thread runs at low priority and checks files with 1 second intervals.

The solution is implemented in r945.

comment:7 by Dmitry A. Kuminov, 13 years ago

Ah and I tested this implementation on a directory with 10 000 files in it using QFileDialog. As opposed to the standard Qt polling watcher (QPollingFileSystemWatcherEngine, used as a fallback) opening such a directory is quite an usable operation. It starts showing files immediately and after a few seconds of being frozen it drops CPU usage to idle and becomes alive. Polling for changes itself doesn't create any significant load so it's just a single DosQueryPathInfo() call per second -- as opposed to the standard poller that would read the directory contents each second and compare this huge list to the saved copy, making the file dialog completely unusable and with constant 100% CPU load.

Note that the fact that the file dialog gets still frozen initially for some seconds is not related to file watching at all: it's because QFileDialog is getting info for 10 000 files using QFileInfo (stat()) in a tight loop which is kind of time consuming. Perhaps needs to be optimized one day but that's a different story.

comment:8 by Dmitry A. Kuminov, 13 years ago

There is one really tricky use case that doesn't work. If you open a file in Qt Creator, rename this file externally, then switch to Qt Creator, it notices the deletion of the original file and asks for an action. However, if you rename the file back before switching to Qt Creator, it doesn't notice the deletion -- which is expected. The unexpected part is that file change notification gets broken after it. I.e. if I change the file externally, Qt Creator doesn't see the changes any longer (though it did see them before). I need to check it out. Tomorrow.

comment:9 by rudi, 13 years ago

While you are into that: There is not method QMutexLocker::locked() that is used in some Q_ASSERTS.

Version 0, edited 13 years ago by rudi (next)

comment:10 by Dmitry A. Kuminov, 13 years ago

Resolution: fixed
Status: newclosed

Turned out to be a Qt Creator bug indeed. Fixed in https://github.com/dmik/qt-creator-os2/commit/2e8b8283a537a14e29cede695767ca309fcabbaa.

I also increased the polling interval of the second thread to 2 seconds in r948. For practical needs this should be enough while less polling is always better (saves system resources).

The debug build break is also fixed, thx for noticing.

The issue should be closed now.

Note: See TracTickets for help on using tickets.