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

[Frank Reininghaus]

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

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/20130806/47e6d630/attachment.htm>