Opened 8 years ago

Closed 8 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 8 years ago.
Possible fix

Download all attachments as: .zip

Change History (11)

Changed 8 years ago by rudi

Possible fix

comment:1 Changed 8 years ago by rudi

  • Component changed from General to QtCore
  • Milestone changed from Qt Enhanced to Qt 4.7
  • Version 4.6.3 deleted

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

comment:2 Changed 8 years ago by dmik

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 Changed 8 years ago by dmik

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 Changed 8 years ago by dmik

The first part is done in r942.

comment:5 Changed 8 years ago by dmik

The second part is done in r944.

comment:6 Changed 8 years ago by dmik

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 Changed 8 years ago by dmik

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 Changed 8 years ago by dmik

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 Changed 8 years ago by rudi

While you are into that: There is no method QMutexLocker::locked(). This causes the debug build to fail due to some Q_ASSERTS.

Last edited 8 years ago by rudi (previous) (diff)

comment:10 Changed 8 years ago by dmik

  • Resolution set to fixed
  • Status changed from new to closed

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.