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

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> +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);
> +	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;
> +}

The design to assume that the majority of objects should be in the
packfiles and the number of loose objects can be ignored when we are
getting approximation is inherited from the world before this
series, I think, which is a valid choice for this series to make.

As your "get an approximate count of loose objects" counts a single
shared fully, instead of punting as soon as the limit is hit, we
could ask that function and add it in when the APPROXIMATE flag is
passed, and get a bit more accurate number cheaply even when we are
approximating.  I am not sure what the pros and cons of doing so
myself, but you may already have thought about it and rejected it,
perhaps?

Thanks for a pleasant read.
Queued.