Review Request 115064: Use only one "directory contents counting" thread per process

Mark Gaiser markg85 at gmail.com
Fri Jan 17 14:10:48 GMT 2014


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115064/#review47571
-----------------------------------------------------------


+1 (as in looks good and sane to me. Not tested it though)

However! (i always seems to have a "however" in those review requests where i do respond :p)
It really smells like you need to use a std::shared_ptr. What you do right now "looks" like manual reference counting and destroying if it's 0. Well, that's about the description of shared pointers as well :)

- Mark Gaiser


On Jan. 17, 2014, 8:23 a.m., Frank Reininghaus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115064/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2014, 8:23 a.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> -------
> 
> Since https://git.reviewboard.kde.org/r/111920/, we use a separate thread for counting the items inside directories (e.g., for the "Size" column in Details View) to prevent that the GUI is blocked while handling large directories. However, this means that Dolphin now uses quite a lot of threads if there are many views. To see that, run Dolphin in gdb, open many tabs with Ctrl+T, and check the output of "thread apply all backtrace.
> 
> I propose to make all views share the same thread. The QThread object is stored in a global variable, and each view increments/decrements a reference count when it starts/stops using the thread. If this thread reaches zero, the thread is stopped.
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/private/kdirectorycontentscounter.h 425c363 
>   dolphin/src/kitemviews/private/kdirectorycontentscounter.cpp fd8479f 
> 
> Diff: https://git.reviewboard.kde.org/r/115064/diff/
> 
> 
> Testing
> -------
> 
> Opening new views does not start new threads any more. The "Size" column in Details view still works normally for me.
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

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


More information about the kfm-devel mailing list