Re: [PATCH] refactor Vector

On Sun, Feb 2, 2014 at 10:42 PM, Amos Jeffries <squid3_at_treenet.co.nz> wrote:
> On 2014-02-03 08:06, Kinkie wrote:
>>
>> Hi,
>> the attached patch (merge from lp:~squid/squid/vector-refactor) is
>> an attempt to refactor Vector and its clients so that:
>> - clients of Vector don't break layering
>> - the Vector API more closely matches std::vector
>>
>> The eventual aim is to replace Vector with std::vector, only if some
>> more accurate measurements (in the lp:~squid/squid/vector-to-stdvector
>> branch) show that this wouldn't cause performance degradations.
>>
>> Don't be fooled by the size of the patch; it is big because there's
>> lots of context.
>>
>> Thanks
>
>
> in src/HttpHdrRange.cc
>
> * This pattern happens several times throughout this patch:
> - while (!specs.empty())
> - delete specs.pop_back();
> + while (!specs.empty()) {
> + delete specs.back();
> + specs.pop_back();
> + }
>
> ** std::vector::pop_back() is documented as erasing the element.
> Squid::Vector does not.
> - under correct usage of std::vector the loop and delete X.back() would
> seem to be irrelevant.
> ** the pattern with loop seems to be equivalent to specs.clear() but far
> less efficient. Efficiency is saved here only by Squid::Vector not
> reallocating the items array in pop_back().

Both left in after Alex' comments.

> in src/HttpHeader.cc:
> * please replace C-style casts with C++ casts in lines changed. static_cast
> would be appropriate in several places in this patch, if needed at all.

Done.

> in src/base/Vector.h
> * still need to remove operator +=. There are likely more conversions to
> push_back() hiding as a result.

Done. There were two.

v2 attached.

-- 
    Francesco