FolderWatcher (linux): optimize slotAddFolderRecursive #5245
Conversation
- Instead of putting all the pathes in a big QStringList and then process them, do the processing as we iterate with QDirIterator - Since we know that the parent directory is not ignored, we can simplify the ignore check by not checking the part before (i.e: we call ExcludedFiles::isExcluded with the parent 'path' as base path) This is what speeds up considerably the function. Note that the OWNCLOUD_TEST check is already there in FolderWatcher::pathIsIgnored so the behavior is not changed
|
@ogoffart I haven't read this in detail yet, but wouldn't the change in exclude check break patterns that include the path ("foo/bar/file" wouldn't match if we feed only "file" inside "foo/bar/")? I'm not sure, in general, if we aim to support patterns like that. |
|
@ckamm good point |
|
I'm actually thinking if we should stop supporting this kind of exclusion pattern?
|
|
|
||
| _root = QDir::tempPath() + "/" + "test_" + QString::number(qrand()); |
There was a problem hiding this comment.
Why did you remove the test_1231 subdirectory?
There was a problem hiding this comment.
I'm using QTemporaryDir which take care of that instead of manually creating and destroying the directory.
| if (folder.exists() && !watchedFolders.contains(subfolder)) { | ||
| #ifndef OWNCLOUD_TEST // InotifyWatcherTest is not interested in ignored files and does not link against the folder | ||
| if( _parent->_folder->syncEngine().excludedFiles().isExcluded( | ||
| subfolder, path, _parent->_folder->ignoreHiddenFiles())) { |
There was a problem hiding this comment.
I think for correctness this must always pass in the full folder-relative path. (see other comment)
|
@ckamm I've never seen full absolute paths in the exclude. What is it absolute to? The system root or the sync folder root? |
|
I've created #5275 for discussion about what the desired behavior for exclude patterns even is. |
|
I wrote my personal conclusion as comment on #5275 (comment) |
|
@ogoffart I'd close this? |
do the processing as we iterate with QDirIterator
ignore check by not checking the part before (i.e: we call
ExcludedFiles::isExcluded with the parent 'path' as base path) This is what
speeds up considerably the function.
Note that the OWNCLOUD_TEST check is already there in FolderWatcher::pathIsIgnored
so the behavior is not changed