Subject: Re: [PATCH v2 16/20] builtin/gc.c: guess the size of the revindex

From: Taylor Blau <[email-masked]>
Date: Thu, 14 Jan 2021 11:53:39 -0500

On Wed, Jan 13, 2021 at 10:33:01PM -0800, Junio C Hamano wrote:
> Taylor Blau <[email-masked]> writes:
>
> > 'estimate_repack_memory()' takes into account the amount of memory
> > required to load the reverse index in memory by multiplying the assumed
> > number of objects by the size of the 'revindex_entry' struct.
> >
> > Prepare for hiding the definition of 'struct revindex_entry' by removing
> > a 'sizeof()' of that type from outside of pack-revindex.c. Instead,
> > guess that one off_t and one uint32_t are required per object. Strictly
> > speaking, this is a worse guess than asking for 'sizeof(struct
> > revindex_entry)' directly, since the true size of this struct is 16
> > bytes with padding on the end of the struct in order to align the offset
> > field.
>
> Meaning that we under-estimate by 25%?

In this area, yes. I'm skeptical that this estimate is all that
important, since it doesn't seem to take into account the memory
required to select delta/base candidates [1].

Thanks,
Taylor

[1]: https://lore.kernel.org/git/[email-masked]/