"Sander Striker" <striker@apache.org> writes:
> > > +/*
> > > + * Support structure to implement 'large arrays'.
> > > + */
> >
> > This is some sort of optimisation isn't it? Is there any profiling
> > that shows a) that it is needed, or b) that it works?
>
> Like I said in my post to the list; we need to investigate if we
> can solve this with basic apr types. This was my implementation
> to get something to work quickly.
A comment in the code is better than a email, then when I look at the
code I won't have to ask questions.
> [...]
> >> + newhat = apr_pcalloc(pool, sizeof(svn_diff_hat_t) + 4095 * sizeof(svn_diff_lcs_t *));
> >
> > A magic number! Why? How did you choose it?
>
> I drew a magic circle on the floor, lit the candles, danced until it started to
> rain and then decided on 4096 (-1 for the one already in the struct). This
> is one of the things that would disappear entirely if we can use an apr type
> for this. /me wonders if apr_array would work.
>
> [...]
> > > + size = 4096;
> >
> > Another one, I assume it's related to the previous one...
>
> Yes.
You seem to have missed my point. Putting one such number in the code
is barely acceptable, putting two different but related numbers is
wrong. There should be a single constant #define'd. This should be
accompanied by a comment saying '4096 because ....' or 'this is just a
guess'. How is anyone else supposed to know what is important if you
don't document it?
--
Philip
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Apr 25 15:13:27 2002