From: Bello Olamide Date: Mon, 01 Jun 2026 14:01:35 GMT Subject: Re: [PATCH v3 1/8] environment: move "trust_ctime" into `struct repo_config_values` Message-ID: In-Reply-To: <08efcc49-0db8-49f6-8971-633aa55eb66c@malon.dev> On Thu, May 21, 2026, 5:37 PM Tian Yuchen wrote: > > 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...' noted... > > Thanks, yuchen Thank you, Yuchen.