On Wednesday, May 28, 2014 16:23:31 Mark Gaiser wrote: > On Wed, May 28, 2014 at 9:59 AM, David Faure <fa...@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#include <QByteArray>
#include <QDebug>
#include <QBuffer>
#include <QIODevice>
#include <QDataStream>
int main(int argc, char** argv)
{
QByteArray ba;
int savePoint = 0;
int records = 0;
QDataStream ds(&ba, QIODevice::WriteOnly);
for (int i = 1; i < 21; ++i) {
if (i == 10) {
savePoint = ds.device()->pos();
}
ds << i;
++records;
}
int endPoint = ds.device()->pos();
ds.device()->seek(savePoint);
ds << 0;
ds.device()->seek(endPoint);
ds << 1337;
++records;
QDataStream out(&ba, QIODevice::ReadOnly);
for (int i = 0; i < records; ++i) {
int j;
out >> j;
qDebug() << j;
}
return 0;
}
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
