Choice of constant in libsvn_fs_base

[Daniel Shahaf <da...@elego.de>]

While reviewing r1205726 I came across this pearl:

reps-strings.c-          /* Make a list of all the rep's we need to undeltify this range.
reps-strings.c-             We'll have to read them within this trail anyway, so we might
reps-strings.c-             as well do it once and up front. */
reps-strings.c-          apr_array_header_t *reps =  /* ### what constant here? */
reps-strings.c:            apr_array_make(pool, 666, sizeof(rep));

Isn't that constant a little too large?  Will any existing repository
ever need a chain of deltas that contains >600 elements?  Can we set the
constant to 30?

Re: Choice of constant in libsvn_fs_base

[Hyrum K Wright <hy...@wandisco.com>]

On Thu, Nov 24, 2011 at 4:00 PM, Daniel Shahaf <da...@elego.de> wrote:

> While reviewing r1205726 I came across this pearl:
>
> reps-strings.c-          /* Make a list of all the rep's we need to
> undeltify this range.
> reps-strings.c-             We'll have to read them within this trail
> anyway, so we might
> reps-strings.c-             as well do it once and up front. */
> reps-strings.c-          apr_array_header_t *reps =  /* ### what constant
> here? */
> reps-strings.c:            apr_array_make(pool, 666, sizeof(rep));
>
> Isn't that constant a little too large?  Will any existing repository
> ever need a chain of deltas that contains >600 elements?  Can we set the
> constant to 30?
>

<bikeshed>
Since we use a 64-bit integer for the revision number, and the number of
reps in a delta chain is log N, the upper bound for this value is 65
(counting both ends of the chain).  However, in practice to reach even that
limit, you'd need a commit every second for 5.85e11 years.  I'm optimistic
about Subversion's longevity, but not *that* optimistic. :)
</bikeshed>

-Hyrum



-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/

Re: Choice of constant in libsvn_fs_base

[Branko Čibej <br...@xbc.nu>]

On 25.11.2011 16:24, Julian Foad wrote:
> Daniel Shahaf wrote:
>
>> While reviewing r1205726 I came across this pearl:
>>
>> reps-strings.c-  /* Make a list of all the rep's we need to undeltify this range.
>> reps-strings.c-     We'll have to read them within this trail anyway, so we might
>> reps-strings.c-     as well do it once and up front. */
>> reps-strings.c-  apr_array_header_t *reps =  /* ### what constant here? */
>> reps-strings.c:    apr_array_make(pool, 666, sizeof(rep));
>>
>> Isn't that constant a little too large?  Will any existing repository
>> ever need a chain of deltas that contains >600 elements?  Can we set the
>> constant to 30?
> +1.
>

Oh come now. Leave a bit of fun in the code. You're going to save all of
10k memory by making that change.

-- Brane

Re: Choice of constant in libsvn_fs_base

[Julian Foad <ju...@btopenworld.com>]

Daniel Shahaf wrote:

> While reviewing r1205726 I came across this pearl:
> 
> reps-strings.c-  /* Make a list of all the rep's we need to undeltify this range.
> reps-strings.c-     We'll have to read them within this trail anyway, so we might
> reps-strings.c-     as well do it once and up front. */
> reps-strings.c-  apr_array_header_t *reps =  /* ### what constant here? */
> reps-strings.c:    apr_array_make(pool, 666, sizeof(rep));
> 
> Isn't that constant a little too large?  Will any existing repository
> ever need a chain of deltas that contains >600 elements?  Can we set the
> constant to 30?

+1.

- Julian