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

On Mon, Jul 29, 2013 at 5:32 PM, Alex Rousskov
<rousskov_at_measurement-factory.com> wrote:
> On 07/29/2013 03:02 AM, Kinkie wrote:
>>>>>> On Sat, Jul 27, 2013 at 8:31 PM, Alex Rousskov 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;
>>>>>>> }
>
>
>> The way I envisioned the key guarantee difference between reserve* and
>> cow() is that: reserve* guarantees that it is safe to APPEND after the
>> currently-used area and up to the requested size/space. cow() slso
>> guarantees that it is safe to WRITE over the currently-used area, up
>> to the requested size.
>
> I do not think it is a good idea to limit reserveCapacity() to append
> because absolutely nothing in its name and in the semantics of standard
> STL methods with similar names restrict them to append. If you call it
> reserve*() it has to apply to buffer capacity for any subsequent operation.

Changing names is not unthinkable. After all we are not bound to a standard API.

> cow() is a private method. It's primary goal is ensuring single
> ownership. Its capacity parameter is just a performance optimization:
> When we have to copy, it saves time to enlarge the buffer as needed as
> well. Perhaps that internal optimization creates some confusion,
> prompting these discussions. From the API point of view, you may ignore
> that optimization and think of cow() as a method that ensures single
> ownership by creating a dedicated buffer and copying current content (if
> needed).

Er.. no (again, it may be misnamed, let's focus on its purpose, we'll
see if we need to change the purpose or the name at the end of this
discussion).
The purpose of cow is to ensure writeability, thus no store sharing
(thus single ownership).
It is a way for the client to say "I'm about to write, please copy if
needed". Nowhere the name talks about single ownership, although it is
a necessary consequence.
The responsibility to reAlloc's purpose is to grow the SBuf (BTW: it
currently doesn't enforce that the requested size be bigger than the
SBuf's data; it should. This is a bug).

>> The former is a weaker guarantee, and a possible optimization. What I
>> understand about your request is that you want the stronger guarantee
>> to be always used. It's not a big deal, as the cost is at most some
>> unneeded copies here and there, and code-wise it's very simple. It'd
>> just alias reserveCapacity to cow, and reserveSpace to wrap on top of
>> that.
>> Did I understand correctly? If so, I can do that right away.
>
> I think you understand what I am suggesting, but I am not sure you
> understand why it should be done that way and why there is no
> performance penalty associated with that solution (if other methods are
> implemented and used correctly).

Yes, you are right.

>> The other size is rawSpace(). For that, I'd like to adopt Amos's
>> suggestion and enforce that the requester specify how much storage he
>> needs. If he wants raw access to the data, he better know what he's
>> doing.
>
> Fine with me.
>
>
>> I don't like the idea of using rawSpace for append;
>
> which is rather absurd since rawSpace() is designed to be used for
> appending and can only be used safely for appending.

Doh! You're right, I hardwired my brain to think of an empty SBuf when
calling it, and wasn't seeing what's written in the code.

>> MemBlob::append
>> also does some housekeeping within MemBlob and I'm not keen on the
>> idea shuffling that outside the class itself. It is however immaterial
>> to this discussion, as it's an implementation detail from the SBuf
>> user's point of view.
>
> I do not see any inappropriate housekeeping in MemBlob::append(). It
> just copies data and increments the used buffer size, as it should.
>
>>> 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.
>>
>> It may be useful to document that reserve* methods are meant to be
>> used only in preparation for other SBuf methods (assign, append, setAt
>> etc., not for direct manipulation by the caller.
>
> There is no need to document that because reserve*() methods return
> void, as they should.
>
>
>> To wrap it up, if you agre I will:
>> - alias reserveCapacity(size) to cow(size)
>> - reserveSpace(size) -> reserveCapacity(size+length())
>
> Yes.
>
>
>> - rawSpace(size_type size=npos) -> rawSpace(size_type size)
>
> OK (but irrelevant to this discussion).
>
>
>> And leave everything else as is.
>
> No, you also need to move your optimization from reserveSpace() to
> rawSpace() and use rawSpace() in append(). There is no need to drop that
> very useful optimization!

Okay, I now think I get it. I was focusing on the return pointer from
rawSpace(). In fact, you are suggesting not to use it, but to use
rawSpace for the purpose that is currently reserveSpace()'s, ignore
the return value and just append as is done now.

So to implement this I should:
- rename current reserveSpace to rawSpace, add returning the pointer
as in current rawSpace.
- reimplement reserveSpace as a convenience calling reserveCapacity
- adjust clients of reserveSpace to use rawSpace instead.

If this is correct, I'm OK with it.

Thanks

-- 
    /kinkie