Re: [PATCH v7 7/9] config: make `delta_base_cache_limit` a non-global variable

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

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

Show 10 quoted lines

> @@ -1697,6 +1701,9 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset,
>  	struct unpack_entry_stack_ent *delta_stack = small_delta_stack;
>  	int delta_stack_nr = 0, delta_stack_alloc = UNPACK_ENTRY_STACK_PREALLOC;
>  	int base_from_cache = 0;
> +	unsigned long delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT;
> +
> +	repo_config_get_ulong(r, "core.deltabasecachelimit", &delta_base_cache_limit);
>
>  	write_pack_access_log(p, obj_offset);
>

Hmm. This repo_config_get_ulong() call will look for the configset entry in a hashmap which is faster than parsing the configuration file from scratch every time, but still expensive for my taste in a function as hot as unpack_entry().

Should this also go in the_repository->settings instead? That way we have a single field access instead of a hashmap lookup (with multiple layers of function calls between us and the actual lookup).

Thanks, Taylor