KIO::ListJob::entries emits twice for folders with 100+ entries. Why?

[David Faure]

On Friday 23 November 2012 22:23:24 Mark wrote:
> Here it is: http://paste.kde.org/613670/

This calls gettimeofday for every entry, even when this value isn't needed. 
Can you move this to the locations where it's actually needed, for performance 
reasons?

Even better would be to port this to QDateTime::currentDateTime or 
QTime::currentTime, to make this more portable (to Windows, without kdewin32).
Or to use QTime::elapsed, which might be even simpler to use for this purpose.

> The changes:
> - It's actually doing batching now regardless of the details level
> - Batching is time based only (300ms). So if the listing takes longer
> then 300ms then all entries thus far will be send. That continues till
> all entries are send.

It continues ... i.e. it waits for another 300ms before sending the second 
batch, right? We don't want to send them one by one, that's for sure.

OK so overall this change means there is no need to call totalSize() at the 
beginning of a directory listing. This should be reflected in the SlaveBase 
documentation.

> - and (while at it) simplified file_unix.cpp's FileProtocol::listDir
> function so that it only follows one path for listing folders. It
> still has a check for details in it, but this "looks" cleaner to me.
> And there is no more putting it in a list and iterating over that one
> for the details != 0 case.

1) The comment "fast path, mostly for recursive deletion", should stay under 
if (details==0).

2) Don't add spaces inside parenthesis (...) where there weren't any before. 
Even though old kdelibs code is a bit inconsistent, the goal is to move 
towards the kdelibs coding style - like Qt: no spaces in (...).

3) What happened to the code that was doing a chdir back to the orig dir?

> I tested this out on a folder with 100.000 files in it and in my case
> i do get 2 entries signals now with no detail level provided (thus
> quite a few details).
> I get only one signal with all the entries in it if i pass details as
> 0. Now it behaves as i was expecting it in the first place.

Sounds good.
Please also test:
* the unittests in kdelibs/kio/tests  (in particular jobtest and 
kdirlistertest, but just run them all with "make test")
* a slow remote server

> Do you guys want me to put this on reviewboard or can i just push it
> to 4.10? My patches are never perfect the first time so i guess you
> guys will have some comments first :p

Yep, please put on reviewboard after fixing the above, it will be easier to 
review the new code than to have it intermixed with the old lines in that diff.
I might have more comments then.

-- 
David Faure, faure at kde.org, http://www.davidfaure.fr
Working on KDE, in particular KDE Frameworks 5