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

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.

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

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

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

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

Cheers,

Alex.
Received on Mon Jul 29 2013 - 15:33:17 MDT