From: Tian Yuchen Date: Wed, 11 Feb 2026 16:43:03 GMT Subject: Re: [PATCH] [RFC][GSoC][PATCH] attr: use local repository state in read_attr Message-ID: <83365a16-68c2-4429-926e-3071df3b9bfb@gmail.com> In-Reply-To: On 2/11/26 07:37, Junio C Hamano wrote: > Tian Yuchen writes: > >> Junio C Hamano writes: >> >> >The codepath read_attr() is in is usually not that hot but it is not >> >cheap. >> >> I'm a bit curious—under what circumstances would calling this method >> result in significant performance regression? > Significant? I dunno. > > And quite honestly, I do not care about significance very much in a > case like this. Doing things that do not make sense, like checking > the same configuration variable again and again when you_know_ that > you never switched to a different repository since you last checked, > is simply wrong. It burdens the readers with unnecessary cognitive > load by making them wonder why you do such a nonsensical thing. > > The read_attr() is called during an attr stack construction, which > traverses the directory hierarchy of a single repositry's working > tree (we do not traverse across submodule boundaries), and the same > istate (i.e., index contents) structure is passed around throughout > the callchain. The repository instance at istate->repo may be a > good place to store "am I bare?" bit that is computed just once and > reused whenever we need to know, like in the funcion under > discussion. Hi Junio, I completely agree that computing it once and storing it in 'istate->repo' is the right fix. Optimizing for logical clarity and reducing load is indeed more important than micro-benchmarking here. Thanks for the insight! Regards, Yuchen