Subject: Re: [PATCH v1 2/3] pack-bitmap.c: add "break" statement in "open_pack_bitmap()"

From: Junio C Hamano <[email-masked]>
Date: Thu, 24 Mar 2022 11:40:02 -0700

Teng Long <[email-masked]> writes:

> There will be only one operant ".bitmap" file in repo, so let's
> add "break" statement in "open_pack_bitmap()" when looping all
> the packs in repo.

Googling "operant" only gives a psychology term X-<.

    : behavior (such as bar pressing by a rat to obtain food) that
    operates on the environment to produce rewarding and reinforcing
    effects.

I do not quite get this.  We expect that there may be more than one
.pack in the object store, and that is why we iterate over the packs
get_all_packs() gives us.  Then for each of these packs, we try to
open pack bitmap.

Ah, are you referring to the current limitation that we can have
only one .bitmap file, even when there are many packs in the object
store?

I wonder if that limitation is something we need to hardcode here.

IOW, when we ever extend the feature to allow more than one, we may
have to extend the "struct bitmap_index" so that we can have bitmap
info from more than one on-disk files, and the implementation of
open_pack_bitmap_1() (which is "read bitmap for this one pack" as
opposed to "read bitmpa for this repository" which is this function)
would certainly have to change, but would we need to revert this
change to the function?  If we do not omit this patch, i.e. even if
we know there can be only one pack that may have a corresponding
bitmap but if the code does not know it and has to try git_open()
only to see it fail, how much wasted effort are we talking about?

> Signed-off-by: Teng Long <[email-masked]>
> ---
>  pack-bitmap.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/pack-bitmap.c b/pack-bitmap.c
> index 931219adf0..b1357137bf 100644
> --- a/pack-bitmap.c
> +++ b/pack-bitmap.c
> @@ -484,8 +484,10 @@ static int open_pack_bitmap(struct repository *r,
>  	assert(!bitmap_git->map);
>  
>  	for (p = get_all_packs(r); p; p = p->next) {
> -		if (open_pack_bitmap_1(bitmap_git, p) == 0)
> +		if (open_pack_bitmap_1(bitmap_git, p) == 0) {
>  			ret = 0;
> +			break;
> +		}
>  	}
>  
>  	return ret;