Re: [PATCH v7 0/9] packfile: avoid using the 'the_repository' global variable

From: Taylor Blau <me@ttaylorr.com>
Date: Nov 20, 2024, 22:55 UTC
Message-ID: <Zz5o35FZPFyM5KyL@nand.local>
In-Reply-To: <cover.1731323350.git.karthik.188@gmail.com>

On Mon, Nov 11, 2024 at 12:14:00PM +0100, Karthik Nayak wrote:

Show 10 quoted lines

> Karthik Nayak (9):
>   packfile: add repository to struct `packed_git`
>   packfile: use `repository` from `packed_git` directly
>   packfile: pass `repository` to static function in the file
>   packfile: pass down repository to `odb_pack_name`
>   packfile: pass down repository to `has_object[_kept]_pack`
>   packfile: pass down repository to `for_each_packed_object`
>   config: make `delta_base_cache_limit` a non-global variable
>   config: make `packed_git_(limit|window_size)` non-global variables
>   midx: add repository to `multi_pack_index` struct

I reviewed this round, and think that it is looking very close. There are a couple of typofixes that I and others have noticed, which are minor (but I think in aggregate should merit a reroll).

I did have a concern about the conversion of delta_base_cache_limit to be a non-global variable, since I think we're determining that value from within unpack_entry() in a more expensive manner than is possible.

So I think that merits some investigation, and will likely result in some changes that we should consider before merging.

Karthik: if you do end up rerolling this, please feel free to include
the patch I sent in [1] on top, which should make the maintainer's life
a bit easier than adding another topic dependent upon this one ;-).

Thanks, Taylor

[1]: https://lore.kernel.org/git/884ca9770d1fb1e84962b1f700b1ce4adce6321c.1732142889.git.me@ttaylorr.com/