Re: [PATCH 5/6] odb/source: introduce generic object counting

[Toon Claes <toon@iotcl.com>]

Patrick Steinhardt <ps@pks.im> writes:

> Introduce generic object counting on the object database source level
> with a new backend-specific callback function.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  odb/source-files.c | 30 ++++++++++++++++++++++++++++++
>  odb/source.h       | 27 +++++++++++++++++++++++++++
>  packfile.c         |  4 ++--
>  packfile.h         |  1 +
>  4 files changed, 60 insertions(+), 2 deletions(-)
>
> diff --git a/odb/source-files.c b/odb/source-files.c
> index 14cb9adeca..c08d8993e3 100644
> --- a/odb/source-files.c
> +++ b/odb/source-files.c
> @@ -93,6 +93,35 @@ static int odb_source_files_for_each_object(struct odb_source *source,
>  	return 0;
>  }
>  
> +static int odb_source_files_count_objects(struct odb_source *source,
> +					  enum odb_count_objects_flags flags,
> +					  unsigned long *out)
> +{
> +	struct odb_source_files *files = odb_source_files_downcast(source);

I didn't read the other series this depends on, but good to see
odb_source_files_downcast() BUGs when the source isn't a 'files'
source.

> +	unsigned long count;
> +	int ret;
> +
> +	ret = packfile_store_count_objects(files->packed, flags, &count);
> +	if (ret < 0)
> +		goto out;
> +
> +	if (!(flags & ODB_COUNT_OBJECTS_APPROXIMATE)) {
> +		unsigned long loose_count;
> +
> +		ret = odb_source_loose_count_objects(source, flags, &loose_count);
> +		if (ret < 0)
> +			goto out;
> +
> +		count += loose_count;
> +	}
> +
> +	*out = count;
> +	ret = 0;
> +
> +out:
> +	return ret;
> +}
> +
>  static int odb_source_files_freshen_object(struct odb_source *source,
>  					   const struct object_id *oid)
>  {
> @@ -220,6 +249,7 @@ struct odb_source_files *odb_source_files_new(struct object_database *odb,
>  	files->base.read_object_info = odb_source_files_read_object_info;
>  	files->base.read_object_stream = odb_source_files_read_object_stream;
>  	files->base.for_each_object = odb_source_files_for_each_object;
> +	files->base.count_objects = odb_source_files_count_objects;
>  	files->base.freshen_object = odb_source_files_freshen_object;
>  	files->base.write_object = odb_source_files_write_object;
>  	files->base.write_object_stream = odb_source_files_write_object_stream;
> diff --git a/odb/source.h b/odb/source.h
> index a1fd9dd920..96c906e7a1 100644
> --- a/odb/source.h
> +++ b/odb/source.h
> @@ -142,6 +142,21 @@ struct odb_source {
>  			       void *cb_data,
>  			       unsigned flags);
>  
> +	/*
> +	 * This callback is expected to count objects in the given object
> +	 * database source. The callback function does not have to guarantee
> +	 * that only unique objects are counted. The result shall be assigned
> +	 * to the `out` pointer.
> +	 *
> +	 * Accepts `enum odb_count_objects_flag` flags to alter the behaviour.
> +	 *
> +	 * The callback is expected to return 0 on success, or a negative error
> +	 * code otherwise.
> +	 */
> +	int (*count_objects)(struct odb_source *source,
> +			     enum odb_count_objects_flags flags,
> +			     unsigned long *out);
> +
>  	/*
>  	 * This callback is expected to freshen the given object so that its
>  	 * last access time is set to the current time. This is used to ensure
> @@ -333,6 +348,18 @@ static inline int odb_source_for_each_object(struct odb_source *source,
>  	return source->for_each_object(source, request, cb, cb_data, flags);
>  }
>  
> +/*
> + * Count the number of objects in the given object database source.
> + *
> + * Returns 0 on success, a negative error code otherwise.
> + */
> +static inline int odb_source_count_objects(struct odb_source *source,
> +					   enum odb_count_objects_flags flags,
> +					   unsigned long *out)
> +{
> +	return source->count_objects(source, flags, out);
> +}
> +
>  /*
>   * Freshen an object in the object database by updating its timestamp.
>   * Returns 1 in case the object has been freshened, 0 in case the object does
> diff --git a/packfile.c b/packfile.c
> index 1ee5dd3da3..8ee462303a 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -1102,6 +1102,7 @@ struct packfile_list_entry *packfile_store_get_packs(struct packfile_store *stor
>  }
>  
>  int packfile_store_count_objects(struct packfile_store *store,
> +				 enum odb_count_objects_flags flags UNUSED,
>  				 unsigned long *out)
>  {
>  	struct packfile_list_entry *e;
> @@ -1146,10 +1147,9 @@ unsigned long repo_approximate_object_count(struct repository *r)
>  
>  		odb_prepare_alternates(r->objects);
>  		for (source = r->objects->sources; source; source = source->next) {
> -			struct odb_source_files *files = odb_source_files_downcast(source);
>  			unsigned long c;
>  
> -			if (!packfile_store_count_objects(files->packed, &c))
> +			if (!odb_source_count_objects(source, ODB_COUNT_OBJECTS_APPROXIMATE, &c))
>  				count += c;
>  		}
>  
> diff --git a/packfile.h b/packfile.h
> index 1da8c729cb..74b6bc58c5 100644
> --- a/packfile.h
> +++ b/packfile.h
> @@ -275,6 +275,7 @@ enum kept_pack_type {
>   * Return 0 on success, a negative error code otherwise.
>   */
>  int packfile_store_count_objects(struct packfile_store *store,
> +				 enum odb_count_objects_flags flags,
>  				 unsigned long *out);
>  
>  /*
>
> -- 
> 2.53.0.880.g73c4285caa.dirty
>

Okay.

-- 
Cheers,
Toon