<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Sep 16, 2015 at 1:13 PM, Andrzej Ostruszka <span dir="ltr"><<a href="mailto:andrzej.ostruszka@gmail.com" target="_blank">andrzej.ostruszka@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">On 09/16/2015 08:57 AM, Thiago Macieira wrote:<br>
>>> You could use peek() to search the buffer, then read() exactly as much as<br>
>>> you really need.<br>
>><br>
>> I understand that we are talking about<br>
>> QIODevicePrivateLinearBuffer::peek() here.<br>
><br>
> No, I meant QIODevice::peek()<br>
<br>
</span>First of all - I do not claim that I have mastered Qt internals!<br>
So everything below you should take with a grain of salt :)<br>
<br>
QIODevice::peek() is implemented in terms of QIODevice::read() (which<br>
then uses QIODevicePrivate::read() which does the memcpy()) and later<br>
putting it back.  Pretty convoluted but it really does not matter - I<br>
just assumed the simpler case of direct peek into the buffer.<br>
<span class=""><br>
>> In your approach I'd be copying data twice, but I can eliminate one with<br>
>> peek()ing<br>
>> directly to the output buffer and later skip()ping in successful case.<br>
>> However in case<br>
>> when delimiter is not yet available I'd have unnecessary copying.<br>
><br>
> Then please provide a patch that avoids the unnecessary second copy when the<br>
> read() destination is a null pointer.<br>
<br>
</span>Here I must say that I don't understand.  Let me rephrase what I've said<br>
earlier.  Suppose that there is some data in buffer and I want to know<br>
if delimiter is in it.  Without direct access to this buffer memory I<br>
can only:<br>
1. copy (by peeking()) it to some other place (possibly the final output<br>
buffer)<br>
2. search for delimiter in my copy<br>
3. if found: drop "number of bytes to delimiter" from buffer<br>
<br>
What I'm saying is that in successful case (when delimiter is found)<br>
this sequence is optimal.  However, in case when there is no delimiter<br>
yet in the buffer, operation 1 is wasted (not really needed).  And I<br>
don't see how to avoid it without allowing user to search in the<br>
internal buffer.<br>
<br>
Possible ways to allow for searching in this buffer are (I'm shortening<br>
names here):<br>
A. Add optional param to canReadLine() readLine() (for QIODP or maybe<br>
even QIOD)<br>
B. Add this QIODPLB::countTo() functionality (which does not break or<br>
interfere with anything)<br>
C. Add QIODPLB::data() function to buffer which will return "first" (I'm<br>
talking in terms of QIODPLB implementation)<br>
D. ???<br>
<br>
As for C.  This is another solution - which also does not interfere with<br>
anything.  The amount of data is already available via size().<br>
Then any kind of "packet" termination can be implemented (not just by<br>
single char) on top of it.  Value returned by this QIODPLB::data() is<br>
only invalidated by explicit user reading so this would be pretty safe<br>
too and its only one line of code.<br>
<br>
So what do you think?<br>
<span class=""><br>
>> And while peek()ing I'd have to use the min(max size of my output<br>
>> buffer, size())<br>
>> which would mean that I'd be copying "overlapping" parts more than once.<br>
><br>
> I didn't understand this.<br>
<br>
</span>What I wanted to say is that:<br>
- in operation 1 above the number of bytes to peek need to be specified<br>
- the only reasonable choice - without using some application specific<br>
heuristic - is the amount of bytes already available (lets forget for a<br>
moment possible constrains of my output buffer size)<br>
<br>
Then in situation when in buffer is more then just one "line" available<br>
I would be copying parts of data after first delimiter in order to later<br>
put them back and read them again on next data arrival.<br>
<span class=""><br>
> I think there are easier ways to do this, so I don't think it makes sense to<br>
> accept this suggestion.<br>
<br>
</span>Yes one can peek() at buffer but as I've tried to state this approach<br>
has some drawbacks (from performance point of view).  I can also<br>
understand that maintainers are worried about growing complexity and<br>
maintenance burden but honestly I don't see any growth in these from<br>
options B and C above.  These are internal, no testing for them is<br>
needed, they don't interfere with any other functionality etc.<br>
<br>
So Your Honour, what is the verdict? ;)<br>
<br>
Regards<br>
<span class=""><font color="#888888">Andrzej<br>
</font></span><div class=""><div class="h5"></div></div></blockquote></div></div><div class="gmail_extra"><br></div><div class="gmail_extra">I'm curious, why is QIODevicePrivate::peek (both versions of it) doing:</div><div class="gmail_extra">- calling read (which resizes the internal buffer to whatever is left after reading). It does this via memmove.</div><div class="gmail_extra">- then it restores the buffer to it's original by taking the current buffer and appending what was read. It does this via memcpy.<br></div><div class="gmail_extra"><br></div><div class="gmail_extra">Rather then doing:</div><div class="gmail_extra">- call QIODevicePrivateLinearBuffer::peek (on the buffer object in QIODevicePrivate) and leave the internal buffer untouched? The function is just sitting there [1], but it isn't being used by either QIODevice(Private)::peek function.</div><div class="gmail_extra"><br></div><div class="gmail_extra">It would avoid fiddling with the internal buffer and just copy (a part of) the internal buffer to the user. That is what peek should do according to the documentation.</div><div class="gmail_extra">Peek would the not call QIODevice::read anymore.</div><div class="gmail_extra"><br></div><div class="gmail_extra">Performance wise, this likely makes peek faster (no more read call, no more memmove, no more memcpy on the internal buffer), memory wise there still is the redundant copy from (a part of) the internal buffer to the user side.</div><div class="gmail_extra">Would it be an idea to add a third peek method to QIODevice that returns the const char* from the internal buffer, perhaps offset by a given position if needed?</div><div class="gmail_extra">That would allow for truly "read only" peeks with no copy in between. A "low level peek" if you will.</div><div class="gmail_extra"><br></div><div class="gmail_extra">[1] <a href="http://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/io/qiodevice_p.h#n110">http://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/io/qiodevice_p.h#n110</a> </div></div>