Re: [PATCH v4 8/9] config: make `packed_git_(limit|window_size)` non-global variables
- From
Jeff King <peff@peff.net>
- Date
- Nov 1, 2024, 17:45 UTC
- Message-ID
- <20241101174547.GB2337295@coredump.intra.peff.net>
- In-Reply-To
- <5bbdc7124d58526a7a2d7b3bdc807ddd204a6df1.1730366765.git.karthik.188@gmail.com>
On Thu, Oct 31, 2024 at 10:39:51AM +0100, Karthik Nayak wrote:
Show 12 quoted lines
> @@ -652,20 +688,25 @@ unsigned char *use_pack(struct packed_git *p,
> break;
> }
> if (!win) {
> - size_t window_align = packed_git_window_size / 2;
> + struct packfile_config config = PACKFILE_CONFIG_INIT;
> + size_t window_align;
> off_t len;
>
> + repo_config(p->repo, packfile_config, &config);
> + window_align = config.packed_git_window_size / 2;
> +Parsing config like this is somewhat expensive (remember we're going to hit your callback for every single config key in the system, user, and repo-level config files).
And use_pack() is a relatively hot code path, as we call it any time we need to access bytes from a mapped pack! This "!win" conditional isn't quite as hot, as it only triggers when we establish a new window for a pack. But that still happens at least once per pack, more if we need to move the window around in a big pack, and lots more if we are under memory pressure and need to open/close windows a lot.
I think we need to parse these values once and then store them somewhere with cheaper access. Can we grab them in prepare_repo_settings(), for example, which would cache them? We need a repo struct, but we have one (the same packed_git->repo you are using to call repo_config()).
-Peff