Re: [PATCH 10/24] pack-bitmap: pass `bitmapped_pack` struct to pack-reuse functions

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2313 bytes --]

On Tue, Nov 28, 2023 at 02:08:21PM -0500, Taylor Blau wrote:
[snip]
> @@ -2002,6 +1986,65 @@ int reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,
>  
>  done:
>  	unuse_pack(&w_curs);
> +}
> +
> +int reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,
> +				       struct packed_git **packfile_out,
> +				       uint32_t *entries,
> +				       struct bitmap **reuse_out)
> +{
> +	struct repository *r = the_repository;
> +	struct bitmapped_pack *packs = NULL;
> +	struct bitmap *result = bitmap_git->result;
> +	struct bitmap *reuse;
> +	size_t i;
> +	size_t packs_nr = 0, packs_alloc = 0;
> +	size_t word_alloc;
> +	uint32_t objects_nr = 0;
> +
> +	assert(result);
> +
> +	load_reverse_index(r, bitmap_git);
> +
> +	if (bitmap_is_midx(bitmap_git)) {
> +		for (i = 0; i < bitmap_git->midx->num_packs; i++) {
> +			struct bitmapped_pack pack;
> +			if (nth_bitmapped_pack(r, bitmap_git->midx, &pack, i) < 0) {
> +				warning(_("unable to load pack: '%s', disabling pack-reuse"),
> +					bitmap_git->midx->pack_names[i]);
> +				free(packs);
> +				return -1;
> +			}
> +			if (!pack.bitmap_nr)
> +				continue; /* no objects from this pack */
> +			if (pack.bitmap_pos)
> +				continue; /* not preferred pack */
> +
> +			ALLOC_GROW(packs, packs_nr + 1, packs_alloc);
> +			memcpy(&packs[packs_nr++], &pack, sizeof(pack));
> +
> +			objects_nr += pack.p->num_objects;
> +		}
> +	} else {
> +		ALLOC_GROW(packs, packs_nr + 1, packs_alloc);
> +
> +		packs[packs_nr].p = bitmap_git->pack;
> +		packs[packs_nr].bitmap_pos = 0;
> +		packs[packs_nr].bitmap_nr = bitmap_git->pack->num_objects;
> +		packs[packs_nr].disjoint = 1;
> +
> +		objects_nr = packs[packs_nr++].p->num_objects;
> +	}
> +
> +	word_alloc = objects_nr / BITS_IN_EWORD;
> +	if (objects_nr % BITS_IN_EWORD)
> +		word_alloc++;
> +	reuse = bitmap_word_alloc(word_alloc);
> +
> +	if (packs_nr != 1)
> +		BUG("pack reuse not yet implemented for multiple packs");

Can't it happen that we have no pack here? In the MIDX-case we skip all
packs that either do not have a bitmap or are not preferred. So does it
mean that in reverse, every preferred packfile must have a a bitmap? I'd
think that to not be true in case bitmaps are turned off.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]