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

> On Sun, Mar 4, 2018 at 9:47 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> > On Sat, Mar 3, 2018 at 6:36 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> 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 <sbeller@google.com>
> >> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> >> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> >> ---
> >> 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/20180226085508.GA30343@sigill.intra.peff.net/#t