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

[Mark]

On Thu, Nov 22, 2012 at 11:52 AM, David Faure <faure at kde.org> wrote:
> On Thursday 15 November 2012 19:37:49 Mark wrote:
>> On Thu, Nov 15, 2012 at 6:26 PM, David Faure <faure at kde.org> wrote:
>> > On Thursday 15 November 2012 13:12:27 Mark wrote:
>> >> My bad, the second signal is only emitting the remainder of whatever
>> >> comes after the first signal.
>> >> However, i still have the question as to why it happens at all?
>> >
>> > So that dolphin doesn't keep the user waiting in front of a blank view for
>> > a long time when opening /usr/bin. Better emit the entries in multiple
>> > batches, for incremental rendering.
>>
>> Sorry, but i have to dive in deep.
>>
>> The actual listing (listDir) on /usr/bin is done very rapidly!
>> Actually, all listings are done very rapidly even on remote locations.
>
> I'm pretty sure you can find a remote enough location where listing will come
> in slowly. And there you'll be glad that SlaveBase doesn't buffer all the
> entries until it receives the last one.

Well, just testing out a linux ftp that hosts a bunch of
archlinux/fedora/debian/ubuntu packages will probably do ;)
>
>> Listing isn't the issue. The thing that makes this "seemingly" slow is
>> all the stuff that happens "after" the listing is in. Like determining
>> the mime type, and sorting the massive list. That is what makes it
>> slow.
>
> No, the fact that this is done after the initial listing makes it seemingly
> fast, on the contrary. You can start seeing stuff appear fast, and then
> mimetype determination happens incrementally. Before we had that, things were
> pretty slow.
>
>> Also, funny that you mention dolphin. Since dolphin can't possibly
>> make use of this. Dolphin is sorting the entries thus needs all of
>> them to be in before it can even sort.
>
> This is wrong, as Kevin Krammer replied.

I stand corrected :)
>
>> And even "if" dolphin receives
>> two entries signals (which is the itemsAdded signal from KDirModel)
>> then it certainly won't speed things up, but rather slow things down
>> because it would have to do sorting twice. Once on a small set of data
>> (relatively fast) and once on the full set of data (dog slow).
>
> Not really slower than doing it in one go. Anyway, sorting is just a bit of
> CPU, the chunked entries is for the case of a slow remote server, where some
> milli-seconds of CPU aren't the issue, but many-seconds latency.
>
>> As a last remark regarding your dolphin example. If you open up a
>> large folder in dolphin you will see that dolphin waits till it has
>> all the contents of it. Then it starts displaying those. So in other
>> terms, it doesn't even benefit!
>
> Don't blame KIO if the new views in dolphin misbehave :-)
>
> This used to be working fine with the Model/View based widgets. If something
> regressed with the new hand-written views in dolphin, then dolphin should be
> fixed. To test what KIO was written for initially, use the file dialog instead.
>
>> You can try this out yourself if you want. Go to a console and make a
>> new folder. Then type:
>> touch {1..100000}.txt
>> Now open up your dolphin in that folder and be prepared to wait :)
>
> I'll trust you on this one, but as I said, wrong test case. Find a slow remote
> server on the other side of the planet, and -there- create a few files.
>
>> If i ADD addMetaData("details", "0") i get:
>> 1 signal with the first 100 entries
>> 1 signal with the remainder
>>
>> If i DON'T add that "addMetaData" line and test the code out on a
>> folder that certainly takes about 10 seconds to get all the entries
>> then i'm welcomed by:
>> 1 signal with all entries
>>
>> You see where this is going? Listing the same folder WITH the
>> "addMetaData" line is going a lot more rapid then WITHOUT the
>> "addMetaData" line. Something is very wrong in the time calculation.
>
> I pointed you to the corresponding code already: SlaveBase::listEntry.
> Feel free to debug the logic in that code. I'm happy to be told that it's
> wrong, it's not even my code :-)

Well, i don't know if that specific code path is wrong, but something
certainly is wrong.
>
>> The results i'm getting should not be possible if the timing was
>> correct. Specially considering that the call with "addMetaData" takes
>> noticeable longer to run yet i only observe one entries signal.
>
> IMHO it shouldn't matter much how listEntry decides to buffer the stuff, but if
> you feel that you must fix it (I agree that your findings look a bit odd), then
> please debug the logic in the above method.

-_- I wanted to get rid of the extra signal, not debug the method and
have the additional signals working as intended.. Though i do now
understand why the extra signal is emited in the first place which
does make me come back on that point i made earlier about getting rid
of it. If it works properly then i do see the benefit of it now.
>
>> >> I personally would just like to get rid of the first signal since in my
>> >> case all entries are feeded to a model anyway.
>> >
>> > I see several issues with that sentence.
>> > 1) Models must be able to react to dynamic changes (e.g. the user creates
>> > a
>> > file), so once they do that, they can also react similarly to "here are 10
>> > entries" followed by "here are 10 more entries" (= inserting rows).
>> > 2) Why not just use KDirModel? It handles all this for you.
>>
>> I am actually using both. I made a breadcrumb bar which uses listDir
>> for every "crumb". I don't need a KDirModel there.
>
> OK. But even if you change the logic in listEntry, your code *must* be
> prepared for entries() to be emitted multiple times, on slow remote servers.

That's no issue. I made sure it can handle as much entries signals as
you can throw at it.:)

To clarify: I am debugging SlaveBase::listEntry now in an attempt to
find out where (or even if) the time based logic is wrong. I will
report back in this thread with the results. Yes, trying it out with a
slow remote server as well just to be sure.


On Thu, Nov 22, 2012 at 12:43 PM, Frank Reininghaus
<frank78ac at googlemail.com> wrote:
> 2012/11/22 David Faure:
> [...]
>>> As a last remark regarding your dolphin example. If you open up a
>>> large folder in dolphin you will see that dolphin waits till it has
>>> all the contents of it. Then it starts displaying those. So in other
>>> terms, it doesn't even benefit!
>>
>> Don't blame KIO if the new views in dolphin misbehave :-)
>>
>> This used to be working fine with the Model/View based widgets. If something
>> regressed with the new hand-written views in dolphin, then dolphin should be
>> fixed. To test what KIO was written for initially, use the file dialog instead.
>
> I just looked at the relevant code (not written by myself) to find out
> something about this 'misbehaviour'.

In the dolphin case i would first try to find out how many times you
even get the newItems signal. I'm not quite sure how KDirListener
works internally, but i doubt it sets the metadata details to 0 which
is the only case where i am getting two entries results. If it only
gets one entries signal then i guess you also get only one newItems
signal even where you "should" have received two.
>
> The slot in KFileItemModel which is connected to KDirLister's
> newItems(KFileItemList) signal collects all new items in a sort of
> cache (m_pendingItemsToInsert), rather than actually inserting them
> into the model, until either
>
> a) the completed() signal is received, or
> b) two seconds have passed.
>
> To be honest, I cannot say at the moment what the reason for this
> design decision is. I could imagine that it is to prevent unnecessary
> small, but expensive, updates of model+view if newItems() is emitted
> many times with just one KFileItem each before the completed() signal
> is received. But it seems that this is actually not even possible, if
> I understand correctly?

No, that's not the case. It's batching. Or rather, it "should" under
some odd conditions that i'm debugging now
1 signal with "some" entries
1 signal with the remainder

It's not sending a signal with every new item in a directory listing.
>
> Best regards,
> Frank