Subject: Re: [PATCH 2/2] refs/files: use heuristic to decide whether to repack with `--auto`

From: Patrick Steinhardt <[email-masked]>
Date: Tue, 3 Sep 2024 11:23:00 +0200

On Tue, Sep 03, 2024 at 02:00:16AM -0700, karthik nayak wrote:
> Patrick Steinhardt <[email-masked]> writes:
> > diff --git a/refs/files-backend.c b/refs/files-backend.c
> > index 1cff65f6ae5..261e2b8570f 100644
> > --- a/refs/files-backend.c
> > +++ b/refs/files-backend.c
> > @@ -1311,6 +1311,78 @@ static int should_pack_ref(struct files_ref_store *refs,
> >  	return 0;
> >  }
> >
> > +static size_t fastlog2(size_t sz)
> 
> Nit: We already `reftable/stack_test.c:fastlog2` and `bisect.c:log2i`. I
> wonder if it would be nice to consolidate all of them. But I guess it's
> okay, since the reftable code anyways is isolated from the rest of the
> Git code.

Yeah, the version we have here is in fact an (almost exact) copy of the
function in our reftable codebase. And as you note, we do not want to
deduplicate these such that the reftable library is somewhat isolated
from the rest of the codebase.

I also noticed `log2i()`, but honestly the only reason why I didn't
reuse it is that I had no clue where to put it. There isn't any header
that would be a good fit for it, and creating a new "math.h" header for
a single function felt overblown to me. So I decided to just not bother.
I'm happy to adjust though if somebody has a suggestion for where to put
it.

Patrick