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

On 27/07/2013 11:38 a.m., Alex Rousskov wrote:
> On 07/26/2013 03:27 PM, Kinkie wrote:
>> Resending due to MUA insisting on using HTML email..
>>
>> On Thu, Jul 18, 2013 at 1:06 AM, Alex Rousskov
>> <rousskov_at_measurement-factory.com> wrote:
>>> On 07/04/2013 11:51 PM, Kinkie wrote:
>>>>>> void
>>>>>>>> SBuf::reserveCapacity(size_type minCapacity)
>>>>>>>> {
>>>>>>>> Must(0 <= minCapacity && minCapacity <= maxSize);
>>>>>>>> reserveSpace(minCapacity-length());
>>>>>>>> }
>>>>>> This does not look right. For example, if minCapacity is smaller than
>>>>>> length(), we should do nothing, but will throw instead. I think this
>>>>>> method should look something like this:
>>>>>>
>>>>>> Must(0 <= minCapacity && minCapacity <= maxSize);
>>>>>> cow(minCapacity);
>>>
>>>>>>>> if (store_->canAppend(off_+len_, minSpace)) {
>>>>>>>> debugs(24, 7, "not growing");
>>>>>>>> return;
>>>>>>>> }
>>>>>> AFAICT, reserveSpace() should be implemented using
>>>>>> something like this:
>>>>>>
>>>>>> Must(0 <= minSpace && minSpace <= maxSize - length());
>>>>>> reserveCapacity(length() + minSpace);
>>>
>>>> The comment is badly worded but the code is correct:
>>> I am not sure which code you are referring to here, but I do not think
>>> it is correct for SBuf::reserveCapacity(minCapacity) to throw when
>>> minCapacity is smaller than length().
>>
>> Decoupled the methods, but the other way around:
>> reserveCapacity(totalCapacity) ensures unique ownership,
>> reserveSpace(capacityToAdd) is an optimization to prime the SBuf to
>> receive data.
>> The upper bound in that Must() clause is actually redundant, as in
>> case of a needed reallocation
>> reserveSpace() -> cow() -> reAlloc() which throws a
>> SBufTooBigException if size is too big.
>> As a consequence, I'm removing the upper bound check.
>> Once we change size_type to be unsigned, the lower bound check will be
>> redundant too.
>>
>>>
>>>
>>>> A decision needs to be made if these two methods have different
>>>> purposes or are to be simply different expressions of the same
>>>> underlying concept (as it is now).
>>> Sorry, this question is too abstract for me to answer it -- it feels
>>> like both statements are true: The methods have different purpose but
>>> are manipulating the same underlying concept of available buffer area.
>>> One method deals with the total size of the buffer area, while the other
>>> is concerned with the size of the yet unused buffer area.
>>
>> As they are (were) one of the two is redundant, an in fact it was two
>> lines of code.
>> With the new semantics, reserveSpace is meant to announce to the SBuf
>> "I'm planning to append lots of data, please be prepared", while
>> reserveCapacity means "I'm going to meddle with your internal storage,
>> and I'm going to need this space. Get ready".
> I do not see the difference because "lots of data" is going to be
> appended to "your internal storage" so both are meddling with that
> storage. To me, the difference was simply in argument math: Sometimes it
> is easier to specify the total capacity, sometimes the available space.
>
> The way the two functions are implemented now the difference is more
> than just math. One does not guarantee exclusive ownership and the other
> one does. Your semantics description above does not seem to convey that
> key difference.
>
> Here is the use case I am thinking about:
>
> sbuf.reserveSpace(ioSize);
> bytesRead = read(sbuf.rawSpace(), ioSize);
> sbuf.forceSize(sbuf.size() + bytesRead);
>
> The above case is already handled by rawSpace(), but now I am confused
> why rawSpace() is implemented using unconditional cow() while
> reserveSpace() appears to be optimizing something. That optimization
> seems to be the key here. Perhaps rawSpace() should be deleted and
> reserveCapacity() adjusted to use the same optimization?
>
> reserveSpace(n) ought to be reserveCapacity(content size + n)
>
>
> To summarize,
>
> 1) We need to agree whether the optimization currently residing in
> reserveSpace() should apply to reserveCapacity(). This is the key
> difference between the two methods. Documentation and code should be
> adjusted accordingly.
>
> 2) We probably should not have both reserveSpace() and rawSpace().

What about " char* getReservedRawSpace(size) " instead?

The reservation mandatory when fetching the raw Space,
  * ensure any cow() is done before returning the buffer,
  * ensure it is 0-safe; the space starts with 0-terminator

Amos
Received on Sat Jul 27 2013 - 11:35:19 MDT