From: Patrick Steinhardt <ps@pks.im>
Date: Wed, 11 Mar 2026 13:55:16 GMT
Subject: Re: [PATCH 2/6] packfile: extract logic to count number of objects
Message-ID: <abF0ROcaUpYxdAQq@pks.im>
In-Reply-To: <871phqmtcu.fsf@iotcl.com>

On Wed, Mar 11, 2026 at 01:41:05PM +0100, Toon Claes wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > In a subsequent commit we're about to introduce a new
> > `odb_source_count_objects()` function so that we can make the logic
> > pluggable. Prepare for this change by extracting the logic that we have
> > to count packed objects into a standalone function.
> >
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> >  packfile.c | 45 +++++++++++++++++++++++++++++++++++----------
> >  packfile.h |  9 +++++++++
> >  2 files changed, 44 insertions(+), 10 deletions(-)
> >
> > diff --git a/packfile.c b/packfile.c
> > index 215a23e42b..1ee5dd3da3 100644
> > --- a/packfile.c
> > +++ b/packfile.c
> > @@ -1101,6 +1101,36 @@ struct packfile_list_entry *packfile_store_get_packs(struct packfile_store *stor
> >  	return store->packs.head;
> >  }
> >  
> > +int packfile_store_count_objects(struct packfile_store *store,
> > +				 unsigned long *out)
> > +{
> > +	struct packfile_list_entry *e;
> > +	struct multi_pack_index *m;
> > +	unsigned long count = 0;
> > +	int ret;
> > +
> > +	m = get_multi_pack_index(store->source);
> > +	if (m)
> > +		count += m->num_objects + m->num_objects_in_base;
> 
> To make sure I understand correctly:
> 
> `m->num_objects` indicates how many objects are in the current pack, and
> `m->num_objects_in_base` how many are in it's base (and that accumulates
> what's in the bases of the base?).

Yup, that's exactly right, `num_objects_in_base` is basically recursive
across the bitmap layers.

> > +	for (e = packfile_store_get_packs(store); e; e = e->next) {
> > +		if (e->pack->multi_pack_index)
> > +			continue;
> 
> Because we added the count through the midx already, we skip any
> packfile that's included in the midx.
> 
> But some packfiles are not in the midx so we fall through for those.
> 
> Makes sense.

Correct.

Patrick