Re: [PATCH] Use SBuf for tunnel.cc I/O

Lets take a step aside and cover why the buf2 is a pointer instead of an
separate SBuf.

When we have two separate SBuf:
 * either one can be changed with append() or consume() without altering
the other.
 ** append() alters the MemBlob backing store
 ** consume() does not alter the MemBlob

So if the caller has SBuf sb1 and comm has SBuf sb2 we get the following
behaviour(s)...

comm_read():
 - will copy the values in sb1 into sb2.
 - will sb2.append(), leaving sb1 unchanged.
 - caller MAY sb1.consume(), leaving sb2 with prefix bytes not in sb1.

The existing read handlers deal with this by using memcpy to shuffle the
newly read bytes down the buffer. This is safe with char* as the buffer
has not been realocated - just a waste of time/cycles occasionally.

PROBLEM 1: sb2.append() may reallocate the MemBlob behind sb2.
 This makes the read handler memcpy() hack completely dangerous.
 option a) is to "sb1 = sb2;" instead

PROBLEM 2: sb1.consume() has removed bytes from the main buffer without
altering sb2. This makes option (a) above unusable.
 option b) is to "sb1.append(sb2);" instead

PROBLEM 3: sb2 contains bytes from the older sb1, not just the newely
read bytes. This makes option (b) above unusable.

What are we left with?
 * complexity converting sb2 into an SBuf the size of the free space in
MemBlob backing store. Which is not actually possible without declaring
that free-space as no longer free (its now "used" by sb2).
  ** also does not solve the issues of sb1.consume() after reallocation
of sb2 MemBlob.

* simple solution and ony way to guarantee that data-copy is not
performed is to have comm_read operate on a pointer to sb1.
 The existence guarantees that requires are the same as for char* buffer
pointers. Already coded for and well tested.

comm_write() is a slightly different story. It *is* possible to take sb2
as a copy then the write handler sb1.consume(len) for the written bytes.

However, since read has the "SBuf *buf2" as a pointer using sb2
non-pointer would require a third IoCallback buffer with additional
logics for managing it. In total this third IoCallback buffer type adds
more complexity than we started with.

Also, we loose ability to interlace read(2) with write(2) and make use
of sb2.consume()'d bytes early when load is making Call cycles slow
down. This is not much, but it is gain.

If you can provide a way to avoid all the above problems while
performing comm_read() on a non-pointer IoCallback "SBuf buf2;" I am
very interested.

I have also considered using MemBlob::Pointer or a non-SBuf wrapper
class IoBuf for MemBlob as the buffer. All that requires new API
extensions to SBuf/Tokenizer/MemBlob to handle the full range of I/O
operations (depending on which extensions are done) and converting
to/from SBuf. Which is a lot more complexity for no gain over the
pointer method.

Amos
Received on Sun Jun 01 2014 - 08:37:44 MDT