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 _______________________________________________ squid-dev mailing list [email protected] http://lists.squid-cache.org/listinfo/squid-dev
