Re: [PATCH 1/2] odb/source-loose: extract logic to retrieve subdirectory bitmap

[Toon Claes <toon@iotcl.com>]

Patrick Steinhardt <ps@pks.im> writes:

> The loose object cache tracks which sharding directories have already
> been scanned via a bitmap. The logic to retrieve the bitmap and mask for
> a given object ID is inlined into the function that populates the cache,
> but we're about to add a second function that will need to perform the
> same logic.
>
> Extract the logic into a separate function to prepare for this.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  odb/source-loose.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/odb/source-loose.c b/odb/source-loose.c
> index 50cb388c352..da8f9767257 100644
> --- a/odb/source-loose.c
> +++ b/odb/source-loose.c
> @@ -26,29 +26,39 @@ static int append_loose_object(const struct object_id *oid,
>  	return 0;
>  }
>  
> -static struct oidtree *odb_source_loose_cache(struct odb_source_loose *loose,
> -					      const struct object_id *oid)
> +static uint32_t *subdir_seen_bitmap(struct odb_source_loose *loose,
> +				    const struct object_id *oid,
> +				    uint32_t *mask)
>  {
>  	int subdir_nr = oid->hash[0];
> -	struct strbuf buf = STRBUF_INIT;
>  	size_t word_bits = bitsizeof(loose->subdir_seen[0]);
>  	size_t word_index = subdir_nr / word_bits;
> -	size_t mask = (size_t)1u << (subdir_nr % word_bits);
> -	uint32_t *bitmap;
>  
>  	if (subdir_nr < 0 ||
>  	    (size_t) subdir_nr >= bitsizeof(loose->subdir_seen))
>  		BUG("subdir_nr out of range");
>  
> -	bitmap = &loose->subdir_seen[word_index];
> +	*mask = (size_t)1u << (subdir_nr % word_bits);

So `mask`'s type is changed from `size_t` to `uint32_t`, which is fine
by me, but shall we use `uint32_t` here too?

> +
> +	return &loose->subdir_seen[word_index];
> +}
> +
> +static struct oidtree *odb_source_loose_cache(struct odb_source_loose *loose,
> +					      const struct object_id *oid)
> +{
> +	struct strbuf buf = STRBUF_INIT;
> +	uint32_t *bitmap, mask;
> +
> +	bitmap = subdir_seen_bitmap(loose, oid, &mask);
>  	if (*bitmap & mask)
>  		return loose->cache;
> +
>  	if (!loose->cache) {
>  		ALLOC_ARRAY(loose->cache, 1);
>  		oidtree_init(loose->cache);
>  	}
>  	strbuf_addstr(&buf, loose->base.path);
> -	for_each_file_in_obj_subdir(subdir_nr, &buf,
> +	for_each_file_in_obj_subdir(oid->hash[0], &buf,
>  				    loose->base.odb->repo->hash_algo,
>  				    append_loose_object,
>  				    NULL, NULL,
>
> -- 
> 2.56.0.500.g5d06dc02cd.dirty
>

-- 
Laters,
Toon