Re: [PATCH] refactor Vector

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

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.

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

Amos
Received on Sun Feb 02 2014 - 21:42:53 MST