Re: [PATCH 08/27] pack: move approximate object count to object store

[Stefan Beller <sbeller@google.com>]

On Wed, Feb 21, 2018 at 4:47 PM, Brandon Williams <bmwill@google.com> wrote:
> On 02/20, Stefan Beller 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>
>> ---
>>  object-store.h | 10 +++++++++-
>>  packfile.c     | 11 +++++------
>>  2 files changed, 14 insertions(+), 7 deletions(-)
>>
>> diff --git a/object-store.h b/object-store.h
>> index 6cecba3951..bd1e4fcd8b 100644
>> --- a/object-store.h
>> +++ b/object-store.h
>> @@ -93,6 +93,14 @@ struct raw_object_store {
>>       struct alternate_object_database *alt_odb_list;
>>       struct alternate_object_database **alt_odb_tail;
>>
>> +     /*
>> +      * A fast, rough count of the number of objects in the repository.
>> +      * These two fields are not meant for direct access. Use
>> +      * approximate_object_count() instead.
>> +      */
>> +     unsigned long approximate_object_count;
>> +     unsigned approximate_object_count_valid : 1;
>
> Patch looks fine and is effectively a no-op, though what is the need for
> both of these variables?  Maybe it can be simplified down to just use
> one?  Just musing as its out of the scope of this patch and we probably
> shouldn't try to change that in this series.

I agree we should. It was introduced in e323de4ad7f (sha1_file:
allow sha1_loose_object_info to handle arbitrary repositories, 2017-08-30)
and I think it was seen as a clever optimization trick back then?