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

[Mark]

On Thu, Nov 29, 2012 at 4:34 PM, David Faure <faure at kde.org> wrote:
> On Thursday 29 November 2012 16:04:21 Mark wrote:
>> On Thu, Nov 29, 2012 at 2:23 PM, David Faure <faure at kde.org> wrote:
>> > 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.
>> Good one! I will use QTime::elapsed. The reason for moving
>> gettimeofday to the top is because it was being called "nearly"
>> always. Unless you had less then 100 entries, then it would only be
>> called once in the main else statement. However, this change requires
>> me to know the time difference every time an entry is inserted so i
>> moved it to the top of the function.
>
> Ah, I see.
> OK (still, with QTime::elapsed this will be a lot more readable and
> maintainable anyway).

Which it is using at this moment on my computer :)
Looks a lot cleaner indeed!

>
>> >> 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.
>>
>> In the most negative situation that "might" happen. So if you are on a
>> very slow network drive that for whatever reason only lists 1 entry
>> every 300 ms then you would indeed get 1 signal per entry. Please do
>> note, this is a corner case and "usually" doesn't happen. But i think
>> broadcasting the entries signal every 300 ms with whatever data is
>> there should be how it's done. (the current patch)
>
> Ah, yes, sure that's fine.
> I meant in the case of a fast listing, your paragraph above made me wonder if
> it was going to send everything one by one after the first 300 ms. Now I see
> this isn't the case.
>
>> > 3) What happened to the code that was doing a chdir back to the orig dir?
>>
>> Ehh, i dropped it :p I saw no need for that to change back. The folder
>> is being changed to the one your listing prior to actually listing the
>> folder so i didn't see a need to put that back. Do you?
>
> You should really read the comments in the current code :-)
> Hint, start at "We restore the path later...".

hehehe oke, i removed the comment ;)
Without joking, i added the path restore part back.

But why is it doing:
#if !defined(PATH_MAX) && defined(__GLIBC__)
    char *path_buffer = ::get_current_dir_name();
    const KDEPrivate::CharArrayDeleter path_buffer_deleter(path_buffer);
#else
    char path_buffer[PATH_MAX];
    path_buffer[0] = '\0';
    (void) getcwd(path_buffer, PATH_MAX - 1);
#endif

for getting the current path? Can't i just do QDir::currentPath() for
the getting part, store it in a variable and put it back at the end?

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