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

From: Junio C Hamano <gitster@pobox.com>
To: Elijah Newren <newren@gmail.com>
Cc: Goss Geppert <gg.oss.dev@gmail.com>,
	Git Mailing List <git@vger.kernel.org>,
	christian w <usebees@gmail.com>,
	Derrick Stolee <derrickstolee@github.com>
Subject: Re: [PATCH v2 1/2] dir: consider worktree config in path recursion
Date: Tue, 24 May 2022 12:45:37 -0700	[thread overview]
Message-ID: <xmqqilpuhfz2.fsf@gitster.g> (raw)
In-Reply-To: <CABPp-BGXRzYCvyM38dEUvQ125+VtRu++7L9UiRz98u+1=Lov7A@mail.gmail.com> (Elijah Newren's message of "Tue, 24 May 2022 07:29:09 -0700")

Elijah Newren <newren@gmail.com> writes:

> 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?