Re: [PATCH v3 1/8] environment: move "trust_ctime" into `struct repo_config_values`

Hi Bello!

On 4/24/26 00:54, Olamide Caleb Bello wrote:

The code itself looks great to me, but I have some reservations about the description here (in terms of why trust_ctime is eagerly parsed):

 > `core.trustctime` is parsed eagerly
 > because it is used in low‑level stat‑matching functions
 > (`match_stat_data()`), where a lazy parse could cause unexpected
 > fatal errors and complicate libification efforts.

It's true that if we use repo_config_get_bool() to parse trust_ctime, following the call stack downwards, there is a die() call. The terminate condition is that the configuration does not exist or contains invalid characters.

But I think there is another factor: match_stat_data() is called on a hot path. The following code is implemented in read-cache.c, refresh_index() function:

	for (i = 0; i < istate->cache_nr; i++) {
		...
		new_entry = refresh_cache_ent(istate, ce, options,
					      &cache_errno, &changed,
					      &t2_did_lstat, &t2_did_scan);
		t2_sum_lstat += t2_did_lstat;
		t2_sum_scan += t2_did_scan;
		if (new_entry == ce)
		...

The call chain: refresh_index() -> refresh_cache_ent() -> ie_match_stat() -> ce_match_stat_basic() -> *match_stat_data()*

Therefore, if the variable is lazily parsed, this means there will be a performance regression whenever the index status needs to be checked, e.g. 'git status'.

So, I guess it would be better to extend a bit:

'...where a lazy parse could cause unexpected fatal, and result in a performance regression...'

Thanks, yuchen