[squid-dev] [PATCH] Faster SBuf::append

[Amos Jeffries]

On 7/10/2016 5:01 a.m., Alex Rousskov wrote:
> Hello,
> 
>     The attached optimization patch was inspired be reviewing the
> following code:
> 
>> Parser::parse(const SBuf &aBuf)
> ...
>> if (preservedData_.isEmpty())
>>     preservedData_ = aBuf; // avoid needless memory allocation
>> else
>>     preservedData_.append(aBuf);
> 
> Supporting this kind of optimization automatically was a little trickier
> than I thought, but I think the attached patch does that. After the
> patch, the above code will collapse to a single append call:
> 
>       preservedData_.append(aBuf);
> 

Please add a check to the unit test testSBuf::testAppendSBuf()
to guarantee that the (*this = S) assignment code path updates the store
reference count rather than doing a bit-wise copy of the SBuf.

> 
> HTH,
> 
> Alex.
> P.S. AFAICT, there is no guarantee that SBuf using GetStorePrototype()
> isEmpty so checking both conditions seems necessary to me.
> 

That sounds like a bug to me. If the initial prototype is anything but
empty then the resulting new/clear'ed SBuf might end up with some random
data prefix. Followup patch?

Amos