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

[Brandon Williams <bmwill@google.com>]

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.

> +
>  	/*
>  	 * Whether packed_git has already been populated with this repository's
>  	 * packs.
> @@ -107,7 +115,7 @@ struct raw_object_store {
>   * that macro on member variables. Use NULL instead as that is defined
>   * and accepted, deferring the real init to prepare_packed_git_mru(). */
>  #define __MRU_INIT { NULL, NULL }
> -#define RAW_OBJECT_STORE_INIT { NULL, NULL, __MRU_INIT, NULL, NULL, 0 }
> +#define RAW_OBJECT_STORE_INIT { NULL, NULL, __MRU_INIT, NULL, NULL, 0, 0, 0 }
>  
>  void raw_object_store_clear(struct raw_object_store *o);
>  
> diff --git a/packfile.c b/packfile.c
> index a8a2e7fe43..693bafbc98 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -803,8 +803,6 @@ static void prepare_packed_git_one(char *objdir, int local)
>  	strbuf_release(&path);
>  }
>  
> -static int approximate_object_count_valid;
> -
>  /*
>   * Give a fast, rough count of the number of objects in the repository. This
>   * ignores loose objects completely. If you have a lot of them, then either
> @@ -814,8 +812,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();
> @@ -825,8 +823,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;
>  }
>  
>  static void *get_next_packed_git(const void *p)
> @@ -901,7 +900,7 @@ void prepare_packed_git(void)
>  
>  void reprepare_packed_git(void)
>  {
> -	approximate_object_count_valid = 0;
> +	the_repository->objects.approximate_object_count_valid = 0;
>  	the_repository->objects.packed_git_initialized = 0;
>  	prepare_packed_git();
>  }
> -- 
> 2.16.1.291.g4437f3f132-goog
> 

-- 
Brandon Williams