Subject: Re: Not going beyond symbolic links

From: Junio C Hamano <[email-masked]>
Date: Fri, 08 Aug 2008 13:55:30 -0700

Junio C Hamano <[email-masked]> writes:

> Linus Torvalds <[email-masked]> writes:
>
>> On Mon, 4 Aug 2008, Junio C Hamano wrote:
>>> 
>>> I started to revisit this issue and patched "git update-index --add"
>>> and "git add" so far.  Patches follow.
>>
>> Patches look good to me, but did you check the performance impact?
>>
>> The rewritten 'has_symlink_leading_path()' should do ok, but it migth 
>> still be a huge performance downside to check all the paths for things 
>> like "git add -u".
>
> I couldn't quite measure much meaningful performance impact.
>
> My test repository was the kernel tree at v2.6.27-rc2-20-g685d87f, without
> any build products nor editor temporary crufts.
>
> By the way, if anybody wants to reproduce this, be careful with the tests
> that run "rm -f .git/index" before adding everything.  After doing so, if
> you check the result with "git diff --stat HEAD", you will notice many
> missing files --- I almost got a heart attack before inspecting this file:
>
> 	$ cat arch/powerpc/.gitignore
>         include
>
> Yes, it excludes 261 already tracked files.  Is it intended?  I dunno.
> ...

Seeing that you applied my arch/powerpc/.gitignore patch to the kernel
(Yaay, I now have a short-log entry in the kernel history ;-), you have
seen the message with some benchmarks I am replying to as well?

I actually was expecting to see measurable slowdown by checking leading
symbolic link more often, but I couldn't, which means either there is not
much downside that the bugfix would hurt real-life usage, or I was too
incompetent to come up with an example to show that the bugfix actually
hurts the performance.

If the former, I think it is a very good news (If the latter, well, we
have a bigger problem :-<), as the affected codepaths are exactly the ones
that need to be fixed when somebody tries to add files in a submodule
directory by mistake, which is an issue (iirc) Dscho had a separate patch
to fix earlier.  Tracked symbolic links may be rare, but it seems that
many people are using submodules, and that case cannot be hand-waved
around by calling it is a feature.