Thread (175 messages) 175 messages, 5 authors, 2024-12-03

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` 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).
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

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help