[Development] Enhancement to QIODevice?

[Thiago Macieira]

On Monday 21 September 2015 12:32:36 Andrzej Ostruszka wrote:
> > Well, yeah, it needs to read at least one from the lower device into the
> > QIODevice buffer. It will then copy the data again to your buffer. There's
> > no way to avoid the double copy, unless device implementation
> > (QSerialPort?) already read into the QIODevice buffer. Please talk to
> > Alex Trotsenko to optimise this.
> > 
> > But canReadLine() needs to do something similar already. If the data isn't
> > buffered yet, it needs to call readData() to copy into the buffer first,
> > then read it.
> 
> This is why I talked about peek() from the buffer itself since I
> wanted to avoid reading from device.

We're talking about two different things here. I was talking about read() and 
peek(), which functions need to readData() from the device in order to do any 
scanning of any kind. Your suggestion will not change this.

You're talking about the user's point of view.

> >> C. Add QIODPLB::data() function to buffer which will return "first" (I'm
> >> talking in terms of QIODPLB implementation)
> > 
> > Returning access to the internal buffer is something to consider. It would
> > allow for one memcpy fewer of the data. But it's not something that we can
> > trivially do.
> 
> [...]
> 
> > The internal buffer is chunked, so the delimiter may not be in the first
> > buffer. And I don't want to provide access to the internal buffers
> > without a lot of thought to design, since after that it becomes ABI and
> > we will have less freedom to improve.
> 
> Is it really chunked?  As I've written previously I'm not wearing
> "expert" hat here but QIODevicePrivateLinearBuffer seems to be a nice
> and simple linear buffer (with data remaining after read() being moved
> to the beginning of buffer).  This buffer is then used in
> QIODevicePrivate which is then used in QIODevice.

You're right about QIODevicePrivate, but that's not the buffer used in QProcess 
and QAbstractSocket. With Alex Trotsenko's work, we could change all of them 
to QRingBuffer.

The point is: I want to be able to assume it's chunked. Therefore, returning 
the buffer is a no-no until we think a lot about it.

> And in case of QSerialPort on unix reading goes like:
> 1. Notification from QSocketNotifier ready to read is passed to
> QSerialPortPrivate::readNotification()
> 2. There, buffer.reserve() returns pointer to write to and we read
> from port to it

Nothing there says it cannot be chunked. You've just described QAbstractSocket 
too, but that one is chunked.

> Similar thing is done in case of windows implementation - we schedule
> asynchronous read to some internal "readChunkBuffer" and in completion
> of this operation we append what was read to the
> QIODevicePrivateLinearBuffer.

And this one sounds like QProcess, except that QWindowsPipeReader is an order 
of magnitude more complex.

> If my understanding is correct then I see no problem in adding simple:
> 
> const char* QIODevicePrivateLinearBuffer::data() const { return first; }

It's not that simple.

Please replace QIODevicePrivateLinearBuffer with QRingBuffer and send your reply 
again.

> > QIODevice needs a complete redesign to support zero-copy. The current API
> > created in Qt 4 (readData(), writeData(), etc.) is way too limited. The
> > problem is that there's no way to do that without a major break in
> > compatibility (so, Qt 6) and spending a *lot* of time fixing all the
> > QIODevice subclasses (QBuffer, QFileDevice, QAbstractSocket, QProcess,
> > QNetworkReply, QSerialPort, plus all the non-Qt code).
> 
> I don't want to suggest that significant change.  I mean I don't
> object any such attempt but I think I'd be satisfied with this little
> addition above.

Yeah, but your little addition requires the thinking ahead that I am talking 
about.

> > I understand the value of countTo(), but it can spiral out of control
> > really quickly. After we add the one containing a single byte delimiter,
> > we'll get> 
> > people asking for:
> >  - string searching (like CRLF)
> >  - regular expression
> >  - state machine searching (CSV: comma, unless inside a "")
> 
> That's why I prefer now just opening internal buffer implementation to
> allow for read-only access to data already buffered instead of my
> original countTo() suggestion.  Any such custom request can be built
> by the user by subclassing QSerialPort/... and accessing data
> available in buffer.

And I'd like buffer access, but I am not going to allow it until we think 
raelly well about future compatibility.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center