Re: [PATCH v2 1/2] dir: consider worktree config in path recursion

From: Junio C Hamano <gitster@pobox.com>
Date: May 24, 2022, 19:45 UTC
Message-ID: <xmqqilpuhfz2.fsf@gitster.g>
In-Reply-To: <CABPp-BGXRzYCvyM38dEUvQ125+VtRu++7L9UiRz98u+1=Lov7A@mail.gmail.com>

Elijah Newren <newren@gmail.com> writes:

Show 25 quoted lines

> On Wed, May 11, 2022 at 9:37 AM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Goss Geppert <gg.oss.dev@gmail.com> writes:
>>
>> > diff --git a/dir.c b/dir.c
>> > index f2b0f24210..a1886e61a3 100644
>> > --- a/dir.c
>> > +++ b/dir.c
>> > @@ -1893,9 +1893,31 @@ static enum path_treatment treat_directory(struct dir_struct *dir,
>>
> [...]
>>
>> > +                     real_gitdir = real_pathdup(the_repository->gitdir, 0);
>>
>> This function is repeatedly called during the traversal.
>>
>> How expensive is it to keep calling real_pathdup() on the constant
>> the_repository->gitdir just in case it might be the same as our true
>> GIT_DIR?
>
> I agree that treat_directory is called many times, but this
> real_pathdup() call is inside the "if (nested_repo)" block, so this
> new real_pathdup() invocation should occur very seldom.  Or are you
> worried about cases where users have *very* large numbers of bare
> repositories nested under the working directory?

No. I wasn't worried about anything in particular. I just wanted to get the feel of how deep a thought the patch was backed by by spot checking what was and what was not taken into account when designing the change.

I do not care too much when there are very large numbers of things that cause this codepath to be exercised. Strange situations can be left for later optimization only when they turn up in the real world and prove to be a problem.

By the way, where is a bare repository involved? did you mean non-bare aka worktree-full repository?