Subject: Re: [PATCH 13/44] pack: move approximate object count to object store

From: Brandon Williams <[email-masked]>
Date: Wed, 21 Mar 2018 15:26:56 -0700

On 03/04, Duy Nguyen wrote:
> On Sun, Mar 4, 2018 at 9:47 AM, Eric Sunshine <[email-masked]> wrote:
> > On Sat, Mar 3, 2018 at 6:36 AM, Nguyễn Thái Ngọc Duy <[email-masked]> wrote:
> >> The approximate_object_count() function maintains a rough count of
> >> objects in a repository to estimate how long object name abbreviates
> >> should be.  Object names are scoped to a repository and the
> >> appropriate length may differ by repository, so the object count
> >> should not be global.
> >>
> >> Signed-off-by: Stefan Beller <[email-masked]>
> >> Signed-off-by: Jonathan Nieder <[email-masked]>
> >> Signed-off-by: Nguyễn Thái Ngọc Duy <[email-masked]>
> >> ---
> >> diff --git a/packfile.c b/packfile.c
> >> @@ -813,8 +811,8 @@ static int approximate_object_count_valid;
> >>  unsigned long approximate_object_count(void)
> >>  {
> >> -       static unsigned long count;
> >> -       if (!approximate_object_count_valid) {
> >> +       if (!the_repository->objects.approximate_object_count_valid) {
> >> +               unsigned long count;
> >>                 struct packed_git *p;
> >>
> >>                 prepare_packed_git();
> >> @@ -824,8 +822,9 @@ unsigned long approximate_object_count(void)
> >>                                 continue;
> >>                         count += p->num_objects;
> >>                 }
> >> +               the_repository->objects.approximate_object_count = count;
> >>         }
> >> -       return count;
> >> +       return the_repository->objects.approximate_object_count;
> >>  }
> >> @@ -900,7 +899,7 @@ void prepare_packed_git(void)
> >>  void reprepare_packed_git(void)
> >>  {
> >> -       approximate_object_count_valid = 0;
> >> +       the_repository->objects.approximate_object_count_valid = 0;
> >
> > Not an issue specific to this patch, but where, how, when does
> > 'approximate_object_count_valid' ever get set to anything other than
> > 0? Even in the existing code (without this patch), there doesn't seem
> > to be anyplace which sets this to a non-zero value. Am I missing
> > something obvious (or subtle)?
> 
> Probably related to this
> https://public-inbox.org/git/[email-masked]/#t

Yeah as far as doing a conversion this should be fine, we can come by
and clean it up at a later point.

-- 
Brandon Williams