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
<[email protected]> 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
