Re: [PATCH 08/27] pack: move approximate object count to object store
- From
Stefan Beller <sbeller@google.com>
- Date
- Feb 23, 2018, 22:22 UTC
- Message-ID
- <CAGZ79kahS_mk8gFONUAxBjw7iV=5q1ff-ZgjLnQ7rMw=kBKThw@mail.gmail.com>
- In-Reply-To
- <20180222004710.GF127348@google.com>
On Wed, Feb 21, 2018 at 4:47 PM, Brandon Williams <bmwill@google.com> wrote:
Show 34 quoted lines
> 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?