Re: [PATCH] commit: Optimize number of lstat() calls

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 4, 2017, 07:23 UTC
Message-ID: <xmqqd1ey5shc.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <alpine.DEB.2.20.1702040129430.3496@virtualbox>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 8 quoted lines

>> Besides, leaving the main index not refreshed would mean the user
>> has to pay the refreshing cost when s/he runs other commands "git
>> diff", "git status", etc. after "git commit" for the first time;
>> so...
>
> I am not sure... when you run `git diff` and `git status`, the index is
> refreshed *anyway*, so with the patch under discussion we would save one
> round of lstat() calls, for the same effect.

Yeah, you're right. The only ones that could be affected are plumbing commands, and scripts that use plumbing are expected to be written without relying on the "refreshed"-ness of the index they are given (iow, if they want to rely on, they are expected to refresh first before using plumbing commands). So there is no downside of leaving the index in an unrefreshed state as long as everbody plays by the rule.

Show 5 quoted lines

> I could imagine that there is a third option we should consider, too: only
> lstat() and update the paths that match the pathspec(s) provided on the
> command line (this is the semantic meaning of the --only option, after
> all: "I am only interested in these paths as far as this commit is
> concerned"). What do you think?

I wondered that myself when I read the first message from Matthew and noticed that we always refresh the entire index. But if it is OK to leave the entire index un-refreshed, that would even be simpler ;-)