Re: StringNg review: MemBlob and BodyPipe

On 03/26/2011 03:44 AM, Kinkie wrote:
>>>>> IMVHO body pipelining should go through SBuf, not MemBlob, but that's
>>>>> for body pipelining to work out.
>>>>
>>>> Currently, MemBlob's append-only design prevents efficient message body
>>>> pipelining. Since SBuf relies on MemBlob, SBuf inherits the same problem
>>>> (and adds more problems due to poor used area tracking). We will need to
>>>> decide whether to enhance MemBlob/SBuf to improve pipelining support or
>>>> switch to a pipeline that uses a "bunch of buffers" instead of a single
>>>> one. This is a topic for another thread though.
>>>
>>> Yes. Unfortunately I don't understand what is the use case you have in
>>> mind :(
>>
>> See BodyPipe and BodyPipe::theBuf use specifically.
>>
>>> My original thought was to use SBuf's append/consume pair for this.
>>
>> Unlike MemBuf used by BodyPipe, SBuf::consume() does not free any space and,
>> hence, cannot be used for efficient body pipelining (i.e., using a
>> relatively small buffer to transfer a possibly large body using asynchronous
>> producer and consumer).
>
> I understand now.
> We could probably define some heuristics to handle this case.
> Something like (on top of my mind):
> if after consume() we have
> - a large-ish backing MemBlob (e.g. > 8kb)
> - with a large unused prefix space (e.g. >4kb)
>
> Then we cow().
> Would this enable SBuf to be used for BodyPipes in your opinion?

Yes, probably, but the question is not just how to make SBuf usable for
BodyPipe, but which use case we want to optimize SBuf for.

If we follow the above "consume-causes-memcpy" algorithm, we are
virtually guaranteed to memcpy bytes of any message body that exceeds
I/O buffer size. And copy about N times where N is the content length
divided by the "large unused prefix size". Thus, the
"consume-causes-memcpy" algorithm is bad for body processing
performance. It is debatable whether it is worse than the current
BodyPipe code, but it is unlikely to be much better.

If we optimize for body forwarding needs instead of preserving the
current implementation, we would need to implement a circular buffer so
that consume frees space without memmove or memcpy. This would be a lot
faster and a lot more complex than the current BodyPipe and SBuf code.

Both approaches would have to cow() in consume(), of course, but the
hope is that cow() would be a no-op after the headers are processed.
Still, you are likely to do at least one large memcpy per message just
to keep all the strings pointing to the headers happy.

A third approach is to use a series of buffers instead of a single
buffer. This approach avoids memcpy and avoids cow() in consume(), so it
is the most efficient one. It can be implemented on top of the existing
SBuf so that we do not make SBuf more complex and so that BodyPipe is
usually compatible with other SBuf users (usually avoiding extra
conversion copies).

This third approach requires custom (or inefficient) processing when a
string crosses buffer boundaries though. In practice, such crossings
should be very rare, but we would still need to write code to handle
them (I would opt for inefficient but simple code in this case).

To summarize, we have four options:

current: StringNG is unusable for BodyPipe; must copy to convert to/from
consume-causes-memcpy: lots of memcpy overhead; few StringNG changes
circular-buffer: at least one memcpy overhead; many StringNG changes
many-buffers: usually no memcpy overhead; many StringNG/core additions

We can move to consume-causes-memcpy now, but it adds more SBuf
complexity and heuristic headaches without an obvious performance
advantage. I am not sure, but it may even throw Squid performance back,
and we already have many performance regressions to deal with. It feels
like consume-causes-memcpy may not be worth it.

I think the long-term goal should be the many-buffers design, but I am
not sure. If others agree, we can just accept the fact that BodyPipe and
SBuf are not compatible for now and move on, until the many-buffers
approach is implemented. If StringNG becomes a performance degradation
point because of this, it will have to be introduced after v3.2 becomes
stable.

Thank you,

Alex.
Received on Tue Mar 29 2011 - 18:55:23 MDT