Re: git-diff in a worktree is an order of magnitude slower?

[Junio C Hamano <gitster@pobox.com>]

"D. Ben Knoble" <ben.knoble@gmail.com> writes:

> But the refresh_index_quietly call is guarded by (effectively; the
> actual code uses rev.diffopt.skip_stat_unmatch)
>
>     1 < !!diff_auto_refresh_index

It is not quite that, is it?  In aecbf914 (git-diff: resurrect the
traditional empty "diff --git" behaviour, 2007-08-31), it read more
like

	if (1 < rev.diffopt.skip_stat_unmatch)
		refresh_index_quietly();

where rev.diffopt.skip_stat_unmatch was initialized to 1 if
diff_auto_refresh_index (boolean) is set to true.

Now, cmd_diff() dispatches to various diff backends to compare two
sets (like "a tree object vs the index", "the index vs the working
tree files"), each of which ends with a call to diffcore_std() and
diffcore_flush() to conclude.   In diffcore_std() there is a call
to diffcore_skip_stat_unmatch() ONLY when skip_stat_unmatch member
is set (we initialize it to 1 when auto-refrresh-index is enabled,
as you saw above).  The function is used to squelch the paths that
remain in diff_queued_diff only because they were stat-dirty without
having an actual content change, and _counts_ how many such ghost
changes existed by incrementing the .skip_stat_unmatch counter.

> which dates to aecbf914c4 (git-diff: resurrect the traditional empty
> "diff --git" behaviour, 2007-08-31). On my system that comparison is
> false because the double-negation produces 1
> (diff_auto_refresh_index=1 or the result of git_config_bool). 

Not quite.  It was false because double-negation initializes the
member to 1, which causes a call to diffcore_skip_stat_unmatch()
be made, *and* the diffcore_skip_stat_unmatch() function did not
find any ghost changes, i.e., paths that were only stat-dirty hence
needed a call to refresh_index_quietly().

> So… has that conditional been quietly dead all this time? I can't
> imagine that's right, but…

I initially thought it was an embarrassing thinko, but after seeing
how .skip_stat_unmatch is used as a 1-based counter (i.e., if the
member says 42, it means it saw 41 paths that were stat-dirty but
without actual content change), I do not think so.

Now, it is a different matter if such a "dual" purpose "more than a
simple boolean" counter is a good idea.  Apparently it confused both
of us in this case ;-).

Thanks.