Review Request 126894: Refactor the listjobtest to allow listing of multile paths.

Frank Reininghaus frank78ac at googlemail.com
Wed Jan 27 06:33:05 UTC 2016


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



Thanks Milian for looking at this test. I implemented it quite some time ago as a quick hack that allows to see how changes in the internal data structures of UDSEntry affect the performance and especially the memory usage of KIO::ListJob (which is what applications typically use to obtain UDSEntries). I did it in Qt4 times, when one could not use lambdas in connections. It's great to see the test simplified and extended now :-)

I have two remarks:

* Appending the listed entries to a list was intentional. If they are discarded immediately, then their memory is freed, and it is not possible to see how much memory the entries would consume in an application that lists one or more paths. I see that this original intention of the test is not obvious at all though. I should have added a comment - sorry about that!
* The `std::cin.ignore()` was also intentional. It allows to investigate the memory usage with htop, ps, or other tools while the process is still running. I know that one could also use Valgrind/Massif+massif-visualizer to get a detailed memory usage report (which also contains detailed information about where and when memory was allocated), but this has the disadvantage that the bookkeeping overhead of the memory allocator is not included (or at least it was not last time I checked). The massif method would tell you that an application that allocates 100 MiB in a single block uses more memory than an application that allocates 10 million blocks of 8 bytes each, but this is not true because some bookkeeping overhead is added to each allocation. Each allocation takes at least 32 bytes of memory when using GCC+glibc on a 64-bit system. But I guess that there must be a better way than the `cin.ignore()` hack to easily get the real memory usage.

- Frank Reininghaus


On Jan. 26, 2016, 3:34 nachm., Milian Wolff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126894/
> -----------------------------------------------------------
> 
> (Updated Jan. 26, 2016, 3:34 nachm.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> -------
> 
> With this change it is now possible to list multiple paths
> as defined via the command line.
> 
> While at it, I refactored the code to clean it up:
> 
> - rely on QEventLoopLocker to quit the application once all jobs
>   have finished
> - use a lambda to count the listed entries
> - don't append to a KIO::UDSEntryList to cound the listed entries
> 
> 
> Diffs
> -----
> 
>   tests/listjobtest.cpp 702b09950734894a9f82746d58071225419b4e3f 
> 
> Diff: https://git.reviewboard.kde.org/r/126894/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Milian Wolff
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160127/d6b44bec/attachment.html>


More information about the Kde-frameworks-devel mailing list