On Wed, Aug 26, 2015 at 1:31 PM, Ivan Zhakov <i...@visualsvn.com> wrote:
> On 26 August 2015 at 11:18, Branko Čibej <br...@wandisco.com> wrote: > > On 25.08.2015 23:08, Stefan Fuhrmann wrote: > >>> All right, so I figured that the difference is that apr_array_make does > >>> two allocations compared to one in this code. Still: relying on > >>> knowledge of APR implementation details is a really bad idea. > >> > >> The structure definition of apr_array_header_t is part of > >> the public API, i.e. will never change. > > > > The semantics might, even if the shape of the structure itself doesn't. > > > >>> As it > >>> stands, APR will correctly resize this array if necessary; but there's > >>> no guarantee that, sometime in the future, resizing such arrays would > >>> fail horribly. > >>> > >> Even if that was true, the resize is done outside APR as > >> well - a few lines further down. > > > > Indeed it is ... and that code is essentially a copy-paste from > > apr_tables.c. > > > > > > I still think this kind of performance hack belongs in APR. Users that > > don't have a new-enough APR won't get the performance boost, but on the > > other hand, the kind of bug that started this discussion will stay out > > of our code. > > > > I think we've had our fair share of alignment bugs with all the > > hand-crafted allocations etc.; we may as well stop now. > > > +1. > Given the clear feedback, I reverted the change on /trunk and proposed the good ol' code for backport. For 1.10, I'll replace svn_ra_svn_item_t within the ra_svn code with one that uses a plain C array instead the APR one - we simply don't need their resizing feature etc. -- Stefan^2.
