KIO directory listing - CPU slows down SSD

[Aaron J. Seigo]

On Wednesday, May 28, 2014 16:23:31 Mark Gaiser wrote:
> On Wed, May 28, 2014 at 9:59 AM, David Faure <faure at kde.org> wrote:
> > On Wednesday 14 May 2014 12:50:18 Aaron J. Seigo wrote:
> >> * one entry at a time, serialized
> >> * no changing data once put there
> >> * no getting the data back out
> >> 
> >> the class would then just batch up data into its QByteArray and then
> >> shunt
> >> it  across the wire when needed. no middle men, no 100s of 1000s of
> >> objects
> >> 
> >> and QLists and << operators. just an API sth like:
> >>         startEntry
> >>         addField
> >>         endEntry
> > 
> > I like this idea very much. Mark: see, it's quite close to what we were
> > discussing by email recently. And I hadn't read this yet :)
> > 
> > Note how this is not specific to the ioslave using QT_STATBUFF :)
> 
> To be honest, i was trying to look at the most used case (which is
> kio_file) and optimize that for raw speed.
> But i kinda like the idea of this approach as well since it would work
> on all slaves :) I will try to implement that instead.
> 
> What i really like about this approach is that it could eventually
> phase out UDSEntry from the slave side. Meaning that UDSEntry would be
> fully client side (where this stream would be parsed).

that would be good for performance indeed; optimizing UDSEntry would still 
have value, of course, since the client side would continue to use it. so 
nobody's efforts there would be wasted

> >> internally it could either batch up the fields and then stream them out
> >> when  endEntry is called, or (and this is what i would do personally) on
> >> startEntry it would set up the header but with bad data (e.g. field count
> >> of 0) and then start appending the fields as they come in and increment a
> >> field count variable. when endEntry is called, update the header with the
> >> right field count.
> > 
> > Ah but batching requires a vector again. Or you meant writing into a
> > bytearray and coming back to update the count - that would rely on
> > QDataStream internals I'm afraid (how to know the offset....).

no internals needed. see attached example.

> > But the alternative is to replace the field count with an "end of fields"
> > special field.
> 
> Huh? "batching requires a vector again" .. i don't understand that. It

consider the current code:

void SlaveBase::listEntries(const UDSEntryList &list)
{
    KIO_DATA << (quint32)list.count();

    foreach (const UDSEntry &entry, list) {
        stream << entry;
    }

    send(MSG_LIST_ENTRIES, data);
}


so David means that because you can't seek in the QDataStream that you'd have 
to batch up entries in a temporary data structure so that you could write the 
header first and THEN write all the entries.

thankfully, the base assumption is incorrect :)

> SomeDataStreamClass stream;
> int i = 0
> while(QT_READDIR(...) && i++) {
>   if(i = 200) { // or whatever the threshold is to send a batch of entries

... and before this happens it needs to set the # of entries it is sending. 
something like a finalizeEntries() method. the class could keep state, 
incrementing a variable every time startEntry() is called

also, rather than count based i'd probably make it based on the size of the 
bytearray. smaller entries would mean more entries sent in a batch, larger 
entries might mean less time waiting on the client side for additional items 
(though that is likely to be nullified by the extra overhead of multiple 
read/writes to the socket and subsequent deserialization trips). in any case, 
one can then optimize the size of the transfer 

>      listEntries(stream);
>      // reset stream and i.
>   }
>   startEntry()
>   addField(...)
>   addField(...)
>   ...
>   endEntry()

endEntry would need to do something similar: it would need to seek back into 
the bytearray where the fields started and update the # of fields to retain 
compatibility with the current code which is:

void UDSEntryPrivate::save(QDataStream &s, const UDSEntry &a)
{
    const FieldHash &e = a.d->fields;

    s << e.size();


> }

the point is that before listEntries is called, the # of items will need to be 
placed at the head of the list so then when deserializing on the client side 
the # of entries is known.

in any case, with QDataStream and a QByteArray (as currently used in the code) 
you CAN seek backwards into the stream and overwrite some of the bytes and 
then even seek back to the end. see the attached example to prove this for 
yourself :)

of course, in this case you don't even need to seek back to the end. you just 
need to seek back to the point where the count ought to be in the stream and 
then serialize a sensible value.

this makes it entirely backwards compatible which means minimal changes 
elsewhere in the code which should mean less chance of introducing new bugs. 
keeping the wire protocol stable should be a goal unless there is real reason 
to do otherwise.

of course, since this is about performance ... what is the cost of that 
seeking?

populating a QByteArray via a QDataStream with 104857600 (1024*1024*100) ints 
takes close to 19 seconds on my laptop and results in a QByteArray which is 
400MB large. (i had to use something that large to be able to reliably measure 
results with a naive QTime::elapsed method.) seeking back to the midpoint of 
that monster, changing the value, then seeking to the end and adding a new 
value takes ... less than a millisecond.

to model what this code would actually need to do (updating the number of 
entries at the end of serialization as well as the field count for each entry), 
i added a "seek back, serialize an int, seek forward" (to emulate updating a 
field count) every for every N ints serialized to the bytearray. with N = 6 (or 
17,476,267 field count updates) adds ~4s; N = 60 and N = 100 both add something 
that is within the noise range offered by using QTime::elapsed() to measure 
such things.

soooo.. i think performance wise this should be just fine, seeing as one would 
probably be sending buffers on the order of some 100s of KB rather than 100s of 
MB ;)

-- 
Aaron J. Seigo
-------------- next part --------------
A non-text attachment was scrubbed...
Name: main.cpp
Type: text/x-c++src
Size: 720 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140528/972f6c0f/attachment.cpp>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140528/972f6c0f/attachment.sig>