Re: [PATCH v7 0/9] packfile: avoid using the 'the_repository' global variable
From: karthik nayak <hidden>
Date: 2024-11-21 13:12:06
Taylor Blau [off-list ref] writes:
On Mon, Nov 11, 2024 at 12:14:00PM +0100, Karthik Nayak wrote:quoted
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` structI 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).
Thanks for the review, indeed, I will be re-rolling with the fixes discussed.
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.
Yup, I'll move it to the repository settings struct and this should help alleviate the perf issue.
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 ;-).
That makes sense, I'll add it in.
Thanks, Taylor [1]: https://lore.kernel.org/git/884ca9770d1fb1e84962b1f700b1ce4adce6321c.1732142889.git.me@ttaylorr.com/ (local)
Attachments
- signature.asc [application/pgp-signature] 690 bytes