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

On 07/27/2013 02:57 PM, Kinkie wrote:
> 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;
>>>> }

> 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

Yes, I know. The current code is inconsistent/confusing because
reserveSpace does not guarantee exclusive buffer ownership but
reserveCapacity does. In the fixed design, that inconsistency is removed.

> (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

The change is essentially moving one method body into another. It does
not introduce any new layer breakage AFAICT. Cow() continues to copy
data if needed, as it does now, via reAlloc().

> cow() has a different contract from reserveSpace:

Not sure why you are discussing reserveSpace(). It is just a convenience
wrapper for reserveCapacity().

> it doesn't check if the MemBlob is append-able.

I do not know what "appendable buffer" means exactly in this context,
but reserveCapacity() is not specific to append in the fixed design. It
is about taking control over the buffer and ensuring its minimal size. I
believe that is what your current cow() does as well.

> 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.

Exactly. This is what reserveCapacity() needs.

> 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 current design is inconsistent (similar-sounding methods behave very
differently) and more dangerous (reserveSpace() does not return volatile
raw space to the caller, increasing the changes that the caller will do
something bad before extracting that [now stale] space from the SBuf).
The proposed design solves both problems.

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

If there is no difference to you, please change it because there is a
significant difference to me (which I am apparently failing to convey).

Thank you,

Alex.
Received on Sat Jul 27 2013 - 23:03:25 MDT