Review Request 111920: Count the items inside directories in another thread

[Emmanuel Pescosta]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111920/#review38761
-----------------------------------------------------------

Ship it!


A ship it from my side, so that we have time to test and extend it in master ;)

- Emmanuel Pescosta


On Aug. 6, 2013, 10:13 p.m., Frank Reininghaus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111920/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2013, 10:13 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Description
> -------
> 
> I think that counting the items inside directories (this number is shown, e.g., in the "Size" column in Details View) could be done in another thread. This not only prevents that the GUI freezes while counting many items in a folder on a slow device [1]. Moving the code that is needed for the counting and directory watching to another class also makes KFileItemModelRolesUpdater, which is complex enough already, a bit simpler.
> 
> Some remarks:
> 
> (a) When sorting by "Size", we still count synchronously in the main thread for 200 ms when new items are inserted into the model. If that succeeds, we can show the items in the correct order immediately.
> 
> (b) KDirectoryContentsCounter is a helper class that lives in the GUI thread. It organizes the dir watching and tells KFileItemModelRolesUpdater via a signal about the results of the counting.
> 
> (c) The actual counting is done in a secondary thread by KDirectoryContentsCounterWorker (Hm, the name might be a bit too long). It communicates with KDirectoryContentsCounter via signals and slots. One problem is that the "Worker" needs to know if it should count hidden files/files and folders/etc. First I used 2 bool arguments for that, but then bool parameters don't really make the code easy to read, so I decided to do it with a flags type KDirectoryContentsCounterWorker::Options. Getting that to work correctly with cross-thread signals was surprisingly complicated, but it works now. I'm not really sure any more if it was worth the effort, but I still think that this is slightly better than the two bools.
> 
> (d) There is one change in
> 
> KFileItemModelRolesUpdater::slotDirectoryContentsCountReceived(const QString& path, int count)
> 
> that is similar to the old function
> 
> KFileItemModelRolesUpdater::slotDirWatchDirty(const QString& path)
> 
> which may not be obvious: when telling the model about the "size" of the directory, it disconnects from the model's "itemsChanged" signal. The effect is that no new preview will be generated if an image is added to/removed from a subfolder shown in the view. This "feature" was only available when the "Size" was shown anyway, so it added some inconsistency. Moreover, one consequence was that a "dirty" signal from KDirWatch resulted in counting the folder contents *twice*: once after the signal was received, and another time after the model's itemsChanged signal was received. Therefore, I think that it makes sense to drop this "feature".
> 
> Any comments are welcome.
> 
> 
> [1] This can be simulated by adding something like
> 
>     QElapsedTimer timer;
>     timer.start();
>     while (timer.elapsed() < 5000);
> 
> to 
> 
> int KFileItemModelRolesUpdater::subItemsCount(const QString& path) const
> 
> 
> This addresses bug 318518.
>     http://bugs.kde.org/show_bug.cgi?id=318518
> 
> 
> Diffs
> -----
> 
>   dolphin/src/CMakeLists.txt 6856991 
>   dolphin/src/kitemviews/kfileitemmodelrolesupdater.h 409f098 
>   dolphin/src/kitemviews/kfileitemmodelrolesupdater.cpp 698a6c5 
>   dolphin/src/kitemviews/private/kdirectorycontentscounter.h PRE-CREATION 
>   dolphin/src/kitemviews/private/kdirectorycontentscounter.cpp PRE-CREATION 
>   dolphin/src/kitemviews/private/kdirectorycontentscounterworker.h PRE-CREATION 
>   dolphin/src/kitemviews/private/kdirectorycontentscounterworker.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/111920/diff/
> 
> 
> Testing
> -------
> 
> Scrolling the view always feels smooth, even when the items inside a large directory are being counted.
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20130827/0365134d/attachment.htm>