Opened 16 years ago
Closed 15 years ago
#6 closed task (fixed)
Port QFileSystemWatcher
Reported by: | Dmitry A. Kuminov | Owned by: | |
---|---|---|---|
Priority: | blocker | Milestone: | Qt 4.6.2 |
Component: | QtCore | Version: | 4.5.1 Beta 1 |
Severity: | medium | Keywords: | |
Cc: |
Description
Provide an OS/2 version of the QFileSystemWatcherEngine class.
Change History (15)
comment:1 by , 15 years ago
Milestone: | QtCore Beta → Qt GA |
---|---|
Status: | new → assigned |
comment:2 by , 15 years ago
Milestone: | Qt GA → Qt Enhanced |
---|
This is basically a convenience feature than the core function (i.e. properly written software will keep working normally without the watcher functionality). Therefore moving to the Enhanced mliestone (we have more important things to do for the GA).
comment:3 by , 15 years ago
Priority: | major → minor |
---|
BTW, QFileSystemWatcher includes a cross-platform implementation that watches for the filesystem by polling the specified directories once per second. This implementation is used on OS/2 now and seems to work OK. This makes this task even less important.
comment:4 by , 15 years ago
Severity: | → low |
---|
We should use the file system change notification function provided by XWorkplace (based on DosOpenChangeNotify and friends API). The current polling-based implementation may cause really slow file list population in standard QFileDialog-based file dialogs under heavy CPU load. With a large number of files in the directory (>100), the polling implementation gives ~50% CPU load here which is not nice.
The details on the XWP notification function may be found in the XWorkplace source code (see http://xtracker.netlabs.org/index.php?bug=1002 for a start). When XWP is not available, we will fall back to the polling implementation (because DosOpenChangeNotify API is exclusively used by WPS in this case and we won't get access to it).
It would be nice to have it in 4.6.1 BTW but we'll see how the CUPS thing goes.
comment:5 by , 15 years ago
Milestone: | Qt Enhanced → Qt 4.6.1 |
---|
We need this for 4.6.1 as people keep reporting high CPU load during file dialog manipulation.
comment:6 by , 15 years ago
Priority: | minor → blocker |
---|---|
Severity: | low → medium |
comment:7 by , 15 years ago
I think that I know another reason why polling may give quite high CPU load (besides that it's polling): IIRC, at present, kLIBC reads file EAs (where it stores Unix permissions) each time stat() is called which may have quite a significant impact on the performance -- I recall I read somewhere (libc trunk maybe) that Knut is planning to make it optional for a given file system. I should check if we can disable EA access with the current kLIBC builds.
Another way of getting rid of this performance penalty is to use the DOS API directly instead of LIBC/posix calls for getting file statistics. Not really difficult but requires some time.
comment:8 by , 15 years ago
The code in the ticket#1002 for XWP, can be used to build standalone monitor for File Change API. This was the initial implementation, to allow non-WPS operation. The code has been integrated inside the XWP core by PR (XWP does not use external tools for core functions).
To match current XWP interface, code on ticket will require minor changes to adapt to newer API in XWP.
comment:9 by , 15 years ago
I see but do you really think that it makes sense to support non-WPS operation mode? Is there a lot of people who do not use WPS? I seriously doubt it...
follow-up: 11 comment:10 by , 15 years ago
Yuri, as far as I see only the pipe name has changed. Did I miss something?
Also, I'm having some problems with notifications here. First, I never get RCNF_CHANGED notifications no matter if I just touch the file or also change its contents. Is it a known issue?
Second, I don't get notifications on disk changes in floppy and CD drives. Though this is probably the limitation of the API itself...
comment:11 by , 15 years ago
Replying to dmik:
Yuri, as far as I see only the pipe name has changed. Did I miss something?
This is apparently regarding your statement about the API change in your original impl and in the final impl in XWP.
comment:12 by , 15 years ago
Implemented the XWP-based file watcher in r616. Besides two mentioned limitations, it works much better than the polling watcher: it does not give any additional CPU load and the changes appear in the application more fast.
Note that this watcher is disabled on the trunk (line 302 in qfilesystemwatcher.cpp) because I want rudy to test the recent polling watcher improvements related to watching drive changes (#137). If it proves to be acceptable, I will be able to implement the same for the XWP-based watcher in the same way, even though Dos*ChangeNotify doesn't support it directly.
Note that in rXXX I also had to fix a bug in QFileInfoGatherer that assumed that all filesystems are case sensitive which prevented QFileDialog from seeing changes in directories containing letters.
comment:13 by , 15 years ago
This has been written 3 years ago, I don't remember. Also I don't see notes in code. but I think changed files should be notified.
It is possible PR changed only named pipe, but since you coded it, that's right.
Standalone daemon was not meant for non-WPS operations, but for PRE-WPS operation, e.g. before getting WPS running.
comment:14 by , 15 years ago
Okay, I see :) Thank you for the feedback. I think we don't care about the pre-WPS time right now, so no need for a standalone daemon.
comment:15 by , 15 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
It turned out that checking for drive changes every 3 seconds is very expensive and creates a lot of noise so forget this idea. Later we may still want to implement watching for attached/detached USB sticks and for date/time/attribute changes if there is a real demand.
I enabled the native watcher in r628. Closing the task.
QFileSystemWatcher is not needed for basic Qt classes; it is only used by the QFileInfoGatherer and QileSystemModel classes from the dialogs GUI module which will be worked on later. Therefore, moving to the GA milestone.