Re: [RFC] Time to talk about StringNG merge again?

From: Kinkie <gkinkie_at_gmail.com>
Date: Sat, 27 Jul 2013 22:57:35 +0200

On Sat, Jul 27, 2013 at 10:32 PM, Alex Rousskov
<rousskov_at_measurement-factory.com> wrote:
> On 07/27/2013 01:03 PM, Kinkie wrote:
>> On Sat, Jul 27, 2013 at 8:31 PM, Alex Rousskov wrote:
>>> On 07/27/2013 12:00 PM, Kinkie wrote:
>>>>>> 1a. Reserve total buffer capacity. Ensure exclusive buffer ownership.
>>>>>>
>>>>>> 1b. Reserve buffer space. Ensure exclusive buffer ownership.
>>>>>>
>>>>>> 2. Reserve N space bytes for the caller to append to. No guarantees
>>>>>> regarding buffer ownership are provided.
>
>>> // 1a.
>>> void reserveCapacity(size_type minCap) {
>>> cow(minCap);
>>> }
>>>
>>> // 1b.
>>> void reserveSpace(size_type minSpace) {
>>> // check that the sum below does not exceed size_type
>>> Must(size() <= size_type's limit - minSpace);
>>> reserveCapacity(size() + minSpace);
>>> }
>>>
>>> // 2.
>>> char *rawSpace(size_type minSpace) {
>>> ... your optimization goes here ...
>>> return pointer to space;
>>> }
>
>> I don't understand how this helps with the append() use-case.
>
> append() should call rawSpace(). rawSpace() is optimized.

Current call chain (and functions for this purpose, outlined):
append (various flavors) calls
  lowAppend which calls
    reserveSpace which
      checks if we're at the tail of the used portion of the store
(via store->canAppend())
      reAlloc() if needed // reAlloc is the low-level, unconditional
part of cow()
  (back to lowAppend): tell the MemBlob to append the user-supplied
buffer and adjust internal data

(typing as I think) using rawSpace as you suggest would shuffle the
responsibility of copying data into the MemBlob out of MemBlob itself
- a layering breakage IMO
cow() has a different contract from reserveSpace: it doesn't check if
the MemBlob is append-able. It promises to reAlloc unless 1. there is
one sole handle to the MemBlob and 2. there is enough space in the
Blob for whatever the user requests to add.
I'm not saying that the same results can't be achieved with the
approach you suggest: it's just designed differently, IMO simply with
a stronger layering and different contracts.

>> The most likely useage pattern would be 2, and that forces a
>> potentially unneeded cow()..
>
> rawSpace() does not force that because your optimization should be
> placed there.

Yes.

>> IMO the three methods would be:
>>
>> void reserveCapacity(size_type minCap) {
>> if (needed())
>> cow();
>> }
>
> I assume that cow() already implements the "if needed" part.

It makes a different promise, see above.

>> reserveCapacity(minSpace+length());
>> }
>
> Yes, plus the size_type overflow check.

Sure.

>> char *rawSpace(size_type minSpace) {
>> cow(minSpace+length());
>> return *freespace;
>> }
>
> No, this needs your no-cow optimization instead. That optimization is
> currently in SBuf::reserveSpace() but should be in rawSpace().

I now get it. I fail to see how this is better than what's currently
done, however :(

-- 
    /kinkie
Received on Sat Jul 27 2013 - 20:57:43 MDT

This archive was generated by hypermail 2.2.0 : Sun Jul 28 2013 - 12:00:06 MDT