* git-diff in a worktree is an order of magnitude slower?
@ 2026-06-08 23:36 D. Ben Knoble
2026-06-09 0:11 ` Jeff King
0 siblings, 1 reply; 17+ messages in thread
From: D. Ben Knoble @ 2026-06-08 23:36 UTC (permalink / raw)
To: Git
Hello all,
I'd like to report and offer to help fix what I view as a serious performance
bug:
"git diff --no-ext-diff --quiet" performs about ~10x slower in a secondary
worktree than in the main worktree.
Fortunately, this doesn't seem to extend to "--cached" (and "--no-ext-diff" and
"--quiet" are probably both red-herrings, since it _does_ extend to plain "git
diff").
Here's a short demo in Git:
# git switch -d v2.54.0
# ninja -C build # where my meson-generated build dir is
# git worktree add --detach ../perf-test v2.54.0
# hyperfine -N --warmup 10 './build/bin-wrappers/git diff'
Benchmark 1: ./build/bin-wrappers/git diff
Time (mean ± σ): 3.4 ms ± 0.5 ms [User: 4.2 ms, System: 3.9 ms]
Range (min … max): 2.5 ms … 5.8 ms 677 runs
# pushd ../perf-test
# hyperfine -N --warmup 10 '../git/build/bin-wrappers/git diff'
Benchmark 1: ../git/build/bin-wrappers/git diff
Time (mean ± σ): 223.3 ms ± 10.5 ms [User: 210.4 ms,
System: 19.1 ms]
Range (min … max): 213.5 ms … 243.9 ms 13 runs
I've had a similar experience at $DAYJOB, where a large repo takes ~6ms for the
former and ~650ms for the latter. I noticed because the Bash prompt functions
execute "git diff --no-ext-diff --quiet", and that was (AFAICT) the largest
culprit for a slow shell prompt in a worktree. To squelch that from the prompt,
I have to go down the rabbit hole of the worktree config extension, so I figured
better to fix the slow diff if possible anyway.
2 questions:
1. Is this known, and if so is anybody working on it?
2. How can I help identify problem areas?
A little more
-------------
I've reproduced this as far back as v2.50.0, which is as far back as I could get
the meson build to work with little effort (so I can't rule out that this is an
old regression).
Using "perf record -F 99 -g -- <bin-wrappers/git> diff" in both trees and then
"perf report":
- it looks like the main worktree spends most of it's time in preload_thread,
threaded_has_symlink_leading_path, lstat_cache…
- the worktree spends a lot more time in ie_match_stat, ce_modified_check_fs,
ce_compare_data, index_fd, would_convert_to_git_filter_fd…
Here's the relevant "perf stat":
main tree:
Performance counter stats for './build/bin-wrappers/git diff':
0 context-switches:u # 0,0
cs/sec cs_per_second
0 cpu-migrations:u # 0,0
migrations/sec migrations_per_second
967 page-faults:u # 65036,4
faults/sec page_faults_per_second
14,87 msec task-clock:u # 0,3
CPUs CPUs_utilized
48 616 branch-misses:u # 3,2 %
branch_miss_rate (57,19%)
3 571 630 branches:u # 240,2
M/sec branch_frequency
13 635 411 cpu-cycles:u # 0,9
GHz cycles_frequency
22 120 068 instructions:u # 1,9
instructions insn_per_cycle (85,61%)
3 634 065 stalled-cycles-frontend:u # 0,28
frontend_cycles_idle (9,56%)
0,006860098 seconds time elapsed
0,001364000 seconds user
0,015157000 seconds sys
worktree:
Performance counter stats for '../git/build/bin-wrappers/git diff':
0 context-switches:u # 0,0
cs/sec cs_per_second
0 cpu-migrations:u # 0,0
migrations/sec migrations_per_second
1 585 page-faults:u # 5058,0
faults/sec page_faults_per_second
313,37 msec task-clock:u # 0,9
CPUs CPUs_utilized
2 481 188 branch-misses:u # 1,5 %
branch_miss_rate (48,94%)
168 664 155 branches:u # 538,2
M/sec branch_frequency (51,21%)
1 004 095 217 cpu-cycles:u # 3,2
GHz cycles_frequency (67,74%)
3 864 851 223 instructions:u # 3,9
instructions insn_per_cycle (52,73%)
70 755 234 stalled-cycles-frontend:u # 0,07
frontend_cycles_idle (49,29%)
0,306707634 seconds time elapsed
0,269027000 seconds user
0,045512000 seconds sys
My observations:
- the worktree has ~twice as many page faults and
- executes ~150 times as many instructions (3.8b compared to 23m).
(When I try to run some "perf" stats as root to access other counters, like
syscalls, "git diff" in the worktree says "not a git repository", so I'm not
counting the actual behavior. Ditto with DTrace.)
PS I almost CC'd Peff and Patrick, whose names stood out in "git
shortlog builtin/{worktree,diff}* object-file* | sort -t\( -k2 -g",
but decided they'd be their own best judge of whether they can
understand what's going on? :)
--
D. Ben Knoble
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: git-diff in a worktree is an order of magnitude slower?
2026-06-08 23:36 git-diff in a worktree is an order of magnitude slower? D. Ben Knoble
@ 2026-06-09 0:11 ` Jeff King
2026-06-09 17:15 ` D. Ben Knoble
0 siblings, 1 reply; 17+ messages in thread
From: Jeff King @ 2026-06-09 0:11 UTC (permalink / raw)
To: D. Ben Knoble; +Cc: Git
On Mon, Jun 08, 2026 at 07:36:45PM -0400, D. Ben Knoble wrote:
> I'd like to report and offer to help fix what I view as a serious performance
> bug:
>
> "git diff --no-ext-diff --quiet" performs about ~10x slower in a secondary
> worktree than in the main worktree.
Hmm, I get the opposite effect: it is much faster in the worktree!
I did:
git clone /path/to/linux.git
git -C linux worktree add --detach ../wt
hyperfine -L dir linux,wt 'git -C {dir} diff'
which yielded:
Benchmark 1: git -C linux diff
Time (mean ± σ): 188.9 ms ± 2.5 ms [User: 166.4 ms, System: 130.7 ms]
Range (min … max): 185.5 ms … 194.8 ms 16 runs
Benchmark 2: git -C wt diff
Time (mean ± σ): 20.0 ms ± 1.5 ms [User: 23.4 ms, System: 103.5 ms]
Range (min … max): 17.2 ms … 24.6 ms 132 runs
Summary
git -C wt diff ran
9.43 ± 0.71 times faster than git -C linux diff
Running:
perf record -g git -C wt --no-pager diff
perf record -g git -C linux --no-pager diff
perf diff
implies that the slow case is spending a lot more time computing sha1s.
Which implies that the entries are stat dirty. And indeed, if I run:
git -C linux update-index --refresh
now they both take ~20ms.
I wonder if it's just a racy-git problem? Many files are written in the
same second as the index, so they end up with the same mtimes, and we
have to err on the side of checking the contents.
See Documentation/technical/racy-git.adoc for a larger discussion.
So it is not really about worktrees at all, but just "bad luck" in
generating that initial index (that goes away next time you actually
make an index update that rewrites the whole thing).
I'd have thought USE_NSEC was the default these days, but looks like it
isn't? Try building with that and I'll bet it goes away entirely.
> PS I almost CC'd Peff and Patrick, whose names stood out in "git
> shortlog builtin/{worktree,diff}* object-file* | sort -t\( -k2 -g",
> but decided they'd be their own best judge of whether they can
> understand what's going on? :)
You might be interested in "git shortlog -ns". :)
-Peff
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: git-diff in a worktree is an order of magnitude slower?
2026-06-09 0:11 ` Jeff King
@ 2026-06-09 17:15 ` D. Ben Knoble
2026-06-11 8:55 ` Jeff King
0 siblings, 1 reply; 17+ messages in thread
From: D. Ben Knoble @ 2026-06-09 17:15 UTC (permalink / raw)
To: Jeff King; +Cc: Git
On Mon, Jun 8, 2026 at 8:11 PM Jeff King <peff@peff.net> wrote:
>
> On Mon, Jun 08, 2026 at 07:36:45PM -0400, D. Ben Knoble wrote:
>
> > I'd like to report and offer to help fix what I view as a serious performance
> > bug:
> >
> > "git diff --no-ext-diff --quiet" performs about ~10x slower in a secondary
> > worktree than in the main worktree.
>
> Hmm, I get the opposite effect: it is much faster in the worktree!
>
> I did:
>
> git clone /path/to/linux.git
> git -C linux worktree add --detach ../wt
> hyperfine -L dir linux,wt 'git -C {dir} diff'
>
> which yielded:
>
> Benchmark 1: git -C linux diff
> Time (mean ± σ): 188.9 ms ± 2.5 ms [User: 166.4 ms, System: 130.7 ms]
> Range (min … max): 185.5 ms … 194.8 ms 16 runs
>
> Benchmark 2: git -C wt diff
> Time (mean ± σ): 20.0 ms ± 1.5 ms [User: 23.4 ms, System: 103.5 ms]
> Range (min … max): 17.2 ms … 24.6 ms 132 runs
>
> Summary
> git -C wt diff ran
> 9.43 ± 0.71 times faster than git -C linux diff
>
> Running:
>
> perf record -g git -C wt --no-pager diff
> perf record -g git -C linux --no-pager diff
> perf diff
>
> implies that the slow case is spending a lot more time computing sha1s.
> Which implies that the entries are stat dirty. And indeed, if I run:
>
> git -C linux update-index --refresh
>
> now they both take ~20ms.
Ah, TIL about --refresh. I suppose it could be nice if "git diff"
updated the index in this way, but that sounds like a band-aid. Maybe
creating a fresh worktree should do the equivalent to make sure it's
considered "fresh"?
(This also dropped my timings down to normal.)
At $DAYJOB, I _think_ some version of "git restore <stuff>" ended up
also updating the index.
> I wonder if it's just a racy-git problem? Many files are written in the
> same second as the index, so they end up with the same mtimes, and we
> have to err on the side of checking the contents.
>
> See Documentation/technical/racy-git.adoc for a larger discussion.
>
> So it is not really about worktrees at all, but just "bad luck" in
> generating that initial index (that goes away next time you actually
> make an index update that rewrites the whole thing).
Ah, that makes sense! I'm familiar with the raciness but didn't expect it here.
> I'd have thought USE_NSEC was the default these days, but looks like it
> isn't? Try building with that and I'll bet it goes away entirely.
Thanks, I'll take a look.
I can see on my Macbook that at least Meson does automatically set
either USE_ST_TIMESPEC or NO_NSEC automatically, but has no option to
enabled USE_NSEC and try that. I can probably write that patch (which
I'll do to test), and I can send it along with the "worktree add
should refresh the index" if you think that's an appropriate thing to
do.
> > PS I almost CC'd Peff and Patrick, whose names stood out in "git
> > shortlog builtin/{worktree,diff}* object-file* | sort -t\( -k2 -g",
> > but decided they'd be their own best judge of whether they can
> > understand what's going on? :)
>
> You might be interested in "git shortlog -ns". :)
>
> -Peff
Phew! Yeah, that's much nicer. Thanks! (When typing this out for
email, I even left out the "grep '^[^[:space:]]' |" filter :P)
--
D. Ben Knoble
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: git-diff in a worktree is an order of magnitude slower?
2026-06-09 17:15 ` D. Ben Knoble
@ 2026-06-11 8:55 ` Jeff King
2026-06-11 17:43 ` Junio C Hamano
2026-06-20 15:57 ` D. Ben Knoble
0 siblings, 2 replies; 17+ messages in thread
From: Jeff King @ 2026-06-11 8:55 UTC (permalink / raw)
To: D. Ben Knoble; +Cc: Git
On Tue, Jun 09, 2026 at 01:15:11PM -0400, D. Ben Knoble wrote:
> > Which implies that the entries are stat dirty. And indeed, if I run:
> >
> > git -C linux update-index --refresh
> >
> > now they both take ~20ms.
>
> Ah, TIL about --refresh. I suppose it could be nice if "git diff"
> updated the index in this way, but that sounds like a band-aid. Maybe
> creating a fresh worktree should do the equivalent to make sure it's
> considered "fresh"?
I think "git diff" _does_ refresh the index internally (that's what
takes so long!). I thought we then wrote out the result, but maybe we
don't notice that it needs an update for some reason?
I'm pretty sure "git status" does something similar, though running it
in a slow working tree _does_ seem to make things faster. Maybe it's
more aggressive about doing the update.
I don't think that refreshing after making a worktree would help. The
problem is one of timestamps: we just wrote an index (so it _should_ be
totally up to date), but we err on the side of caution for some entries
because the file timestamps and the index timestamp are the same. So
what makes it "work" is that one second passed between writing those
files and running "update-index". If you ran it from the worktree
command automatically, it might all still happen in the same second.
And of course, it's not just worktrees. Any time we checkout we may
suffer from this problem, though initial clones and worktree creation
will write more files than most.
> At $DAYJOB, I _think_ some version of "git restore <stuff>" ended up
> also updating the index.
Yep, that would make sense. Any index write (after the second-hand
ticks) will make it go away, since it means updating the mtime of the
index.
> > I'd have thought USE_NSEC was the default these days, but looks like it
> > isn't? Try building with that and I'll bet it goes away entirely.
>
> Thanks, I'll take a look.
>
> I can see on my Macbook that at least Meson does automatically set
> either USE_ST_TIMESPEC or NO_NSEC automatically, but has no option to
> enabled USE_NSEC and try that. I can probably write that patch (which
> I'll do to test), and I can send it along with the "worktree add
> should refresh the index" if you think that's an appropriate thing to
> do.
I think NO_NSEC is about not looking at the nsec fields of stat structs
(since they might not exist). But we don't actually use them for stat
matching unless USE_NSEC is set.
I guess the distinction goes back to c06ff4908b (Record ns-timestamps if
possible, but do not use it without USE_NSEC, 2009-03-04), which details
some reasons you might not want USE_NSEC. Feels like it ought to be a
run-time config, though, and maybe even something that gets auto-probed
by git-init.
Definitely not an area I have looked at much, though, nor thought hard
about. So there might be gotchas. :)
-Peff
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: git-diff in a worktree is an order of magnitude slower?
2026-06-11 8:55 ` Jeff King
@ 2026-06-11 17:43 ` Junio C Hamano
2026-06-11 21:06 ` brian m. carlson
2026-06-20 15:57 ` D. Ben Knoble
1 sibling, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2026-06-11 17:43 UTC (permalink / raw)
To: Jeff King; +Cc: D. Ben Knoble, Git
Jeff King <peff@peff.net> writes:
> I guess the distinction goes back to c06ff4908b (Record ns-timestamps if
> possible, but do not use it without USE_NSEC, 2009-03-04), which details
> some reasons you might not want USE_NSEC. Feels like it ought to be a
> run-time config, though, and maybe even something that gets auto-probed
> by git-init.
I thought for a bit but didn't think of a clean way to auto-probe if
a filesystem loses nanosecond-precision part of .st_Xtime when
"metadata is flushed and later read back in" with reasonable
overhead. I do not think we want to trigger system-wide sync and/or
dropping of buffer cache ;-)
> Definitely not an area I have looked at much, though, nor thought hard
> about. So there might be gotchas. :)
>
> -Peff
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: git-diff in a worktree is an order of magnitude slower?
2026-06-11 17:43 ` Junio C Hamano
@ 2026-06-11 21:06 ` brian m. carlson
0 siblings, 0 replies; 17+ messages in thread
From: brian m. carlson @ 2026-06-11 21:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, D. Ben Knoble, Git
[-- Attachment #1: Type: text/plain, Size: 1283 bytes --]
On 2026-06-11 at 17:43:52, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I guess the distinction goes back to c06ff4908b (Record ns-timestamps if
> > possible, but do not use it without USE_NSEC, 2009-03-04), which details
> > some reasons you might not want USE_NSEC. Feels like it ought to be a
> > run-time config, though, and maybe even something that gets auto-probed
> > by git-init.
>
> I thought for a bit but didn't think of a clean way to auto-probe if
> a filesystem loses nanosecond-precision part of .st_Xtime when
> "metadata is flushed and later read back in" with reasonable
> overhead. I do not think we want to trigger system-wide sync and/or
> dropping of buffer cache ;-)
We could have `git update-index` take options like it does for
`--untracked-cache` and `--no-untracked-cache` to control these for
people who want them. For instance, I know what operating system and
file system I'm using (Linux with btrfs), so if I know that option is
safe, I can enable it at runtime and reap the benefits.
We could even have `--test-use-nsec` to perform a `uname` and `statfs`
call to determine whether this is a known safe configuration if probing
is not possible.
--
brian m. carlson (they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: git-diff in a worktree is an order of magnitude slower?
2026-06-11 8:55 ` Jeff King
2026-06-11 17:43 ` Junio C Hamano
@ 2026-06-20 15:57 ` D. Ben Knoble
2026-06-21 0:53 ` Junio C Hamano
1 sibling, 1 reply; 17+ messages in thread
From: D. Ben Knoble @ 2026-06-20 15:57 UTC (permalink / raw)
To: Jeff King; +Cc: Git
Coming back to index refreshing…
On Thu, Jun 11, 2026 at 4:55 AM Jeff King <peff@peff.net> wrote:
>
> On Tue, Jun 09, 2026 at 01:15:11PM -0400, D. Ben Knoble wrote:
>
> > > Which implies that the entries are stat dirty. And indeed, if I run:
> > >
> > > git -C linux update-index --refresh
> > >
> > > now they both take ~20ms.
> >
> > Ah, TIL about --refresh. I suppose it could be nice if "git diff"
> > updated the index in this way, but that sounds like a band-aid. Maybe
> > creating a fresh worktree should do the equivalent to make sure it's
> > considered "fresh"?
>
> I think "git diff" _does_ refresh the index internally (that's what
> takes so long!). I thought we then wrote out the result, but maybe we
> don't notice that it needs an update for some reason?
>
> I'm pretty sure "git status" does something similar, though running it
> in a slow working tree _does_ seem to make things faster. Maybe it's
> more aggressive about doing the update.
Thanks for the status pointer:
- cmd_status calls refresh_index and repo_updated_index_if_able
- those same calls are wrapped in refresh_index_quietly in builtin/diff.c
But the refresh_index_quietly call is guarded by (effectively; the
actual code uses rev.diffopt.skip_stat_unmatch)
1 < !!diff_auto_refresh_index
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). Or at
least, I don't see it get written to elsewhere (maybe that's supposed
to happen in diff.c:diffcore_skip_stat_unmatch in this case and isn't?
Idk. (Even dirtying the worktree as a hypothesis that only when a diff
is found does the counter get bumped doesn't seem to work.)
So… has that conditional been quietly dead all this time? I can't
imagine that's right, but…
> > > I'd have thought USE_NSEC was the default these days, but looks like it
> > > isn't? Try building with that and I'll bet it goes away entirely.
> >
> > Thanks, I'll take a look.
> >
> > I can see on my Macbook that at least Meson does automatically set
> > either USE_ST_TIMESPEC or NO_NSEC automatically, but has no option to
> > enabled USE_NSEC and try that. I can probably write that patch (which
> > I'll do to test), and I can send it along with the "worktree add
> > should refresh the index" if you think that's an appropriate thing to
> > do.
>
> I think NO_NSEC is about not looking at the nsec fields of stat structs
> (since they might not exist). But we don't actually use them for stat
> matching unless USE_NSEC is set.
>
> I guess the distinction goes back to c06ff4908b (Record ns-timestamps if
> possible, but do not use it without USE_NSEC, 2009-03-04), which details
> some reasons you might not want USE_NSEC. Feels like it ought to be a
> run-time config, though, and maybe even something that gets auto-probed
> by git-init.
>
> Definitely not an area I have looked at much, though, nor thought hard
> about. So there might be gotchas. :)
>
> -Peff
Looks like adding USE_NSEC to my build did make the issue go away (the
patch is short, and I'll send it anyway for folks to have the knob),
but that now seems like a band-aid to me based on my confusion above.
--
D. Ben Knoble
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: git-diff in a worktree is an order of magnitude slower?
2026-06-20 15:57 ` D. Ben Knoble
@ 2026-06-21 0:53 ` Junio C Hamano
2026-06-21 3:58 ` Junio C Hamano
2026-06-21 17:24 ` Jeff King
0 siblings, 2 replies; 17+ messages in thread
From: Junio C Hamano @ 2026-06-21 0:53 UTC (permalink / raw)
To: D. Ben Knoble; +Cc: Jeff King, Git
"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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: git-diff in a worktree is an order of magnitude slower?
2026-06-21 0:53 ` Junio C Hamano
@ 2026-06-21 3:58 ` Junio C Hamano
2026-06-21 17:24 ` Jeff King
1 sibling, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2026-06-21 3:58 UTC (permalink / raw)
To: D. Ben Knoble; +Cc: Jeff King, Git
Junio C Hamano <gitster@pobox.com> writes:
> 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 ;-).
FWIW the patch was done as part of this discussion thread:
https://lore.kernel.org/git/20070830063810.GD16312@mellanox.co.il/
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: git-diff in a worktree is an order of magnitude slower?
2026-06-21 0:53 ` Junio C Hamano
2026-06-21 3:58 ` Junio C Hamano
@ 2026-06-21 17:24 ` Jeff King
2026-06-21 17:45 ` Jeff King
1 sibling, 1 reply; 17+ messages in thread
From: Jeff King @ 2026-06-21 17:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: D. Ben Knoble, Git
On Sat, Jun 20, 2026 at 05:53:02PM -0700, Junio C Hamano wrote:
> > 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().
I think this is the core of the issue. These entries are "racy git
dirty" in the sense that their mtimes are the same as the index mtime,
and so we double-check the contents. This is the first bullet point
under the "Racy Git" section of Documentation/technical/racy-git.adoc.
But diffcore_skip_stat_unmatch() doesn't count them as dirty, so we
don't increment the counter, and thus top-level git-diff won't write out
the new index. And thus every subsequent diff repeats the same
expensive double-check.
But I'm not sure where the blame lies. Either:
1. diffcore_skip_stat_unmatch() should be counting these in its
"dirty" counter; or
2. the index should be marking these differently. The second bullet
point of that Racy Git section says:
When the index file is updated that contains racily clean
entries, cached `st_size` information is truncated to zero
before writing a new version of the index file.
Should the index be written out with a 0 size field here, so that
we know they are dirty and should be updated? I guess that would be
user-visible, though, because commands that _don't_ update the
index (like plumbing diff-files) would generate a spurious diff
there rather than doing the content-level comparison.
I dunno. You had solved most of the racy git stuff before I came along,
so I never gave it too much thought (and what little thought I did was
many years ago).
> > 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 ;-).
Make that three of us. ;)
-Peff
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: git-diff in a worktree is an order of magnitude slower?
2026-06-21 17:24 ` Jeff King
@ 2026-06-21 17:45 ` Jeff King
2026-06-21 20:24 ` Junio C Hamano
2026-06-21 21:39 ` Junio C Hamano
0 siblings, 2 replies; 17+ messages in thread
From: Jeff King @ 2026-06-21 17:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: D. Ben Knoble, Git
On Sun, Jun 21, 2026 at 01:24:32PM -0400, Jeff King wrote:
> I think this is the core of the issue. These entries are "racy git
> dirty" in the sense that their mtimes are the same as the index mtime,
> and so we double-check the contents. This is the first bullet point
> under the "Racy Git" section of Documentation/technical/racy-git.adoc.
>
> But diffcore_skip_stat_unmatch() doesn't count them as dirty, so we
> don't increment the counter, and thus top-level git-diff won't write out
> the new index. And thus every subsequent diff repeats the same
> expensive double-check.
>
> But I'm not sure where the blame lies. Either:
>
> 1. diffcore_skip_stat_unmatch() should be counting these in its
> "dirty" counter; or
BTW, I don't think diffcore actually has the information it would need
to do so. The racy stuff is handled under the hood in ie_match_stat(),
which returns only a set of "changed" flags. So the caller cannot tell
the difference between the two cases:
1. We checked ce_match_stat_basic() which said "no change", and then
is_racy_timestamp() was false, so that was good enough.
2. is_racy_timestamp() is true, so we further did a content check,
found nothing, and returned the same "no change"
Obviously we could pass back another flag, but that would disrupt the
other callers. Hmm. It looks like we could pass in a flag to say "assume
racy entries are modified". And then they come back to the diff code,
diffcore_skip_stat_unmatch() sees they're not real diffs and suppresses
them, but we _do_ count them as stat-dirty.
Like this:
diff --git a/builtin/diff.c b/builtin/diff.c
index 4b46e394ce..4d36b5c1e0 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -271,6 +271,9 @@ static void builtin_diff_files(struct rev_info *revs, int argc, const char **arg
argv++; argc--;
}
+ if (revs->diffopt.skip_stat_unmatch)
+ options |= DIFF_RACY_IS_MODIFIED;
+
/*
* "diff --base" should not combine merges because it was not
* asked to. "diff -c" should not densify (if the user wants
That seems to work, in the sense that "git diff" does refresh the index
afterwards. But the timings are a bit funny.
In my working tree of linux.git with many racy entries it was ~500ms to
do the first diff (and the second, and so on, because we never updated
the index). After the patch above, it is 1800ms to do the first diff,
and then fast (~30ms) after.
I could believe it takes twice as long when we refresh the index
(because I don't think we use the stat-cleanliness we collected from the
diff, but rather just do a from-scratch index refresh). But that would
imply it should take ~1000ms. Where does the extra 800ms go? I guess
that somehow the content-check done by diffcore_skip_stat_unmatch() is
slower than the one done by ie_match_stat(). I think the individual
functions are respectively diff_filespec_check_stat_unmatch() and
ce_modified_check_fs().
I don't know if any of this is really worth digging too far. This feels
like a case we could do a bit better at, but I wonder how much it
matters in practice. As soon as you do any index-refresh (including "git
status"), the racy entries are cleared and everything is faster. It
just seems kind of lame that we write out the initial working tree with
so many racy entries.
-Peff
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: git-diff in a worktree is an order of magnitude slower?
2026-06-21 17:45 ` Jeff King
@ 2026-06-21 20:24 ` Junio C Hamano
2026-06-21 21:28 ` Jeff King
2026-06-21 21:39 ` Junio C Hamano
1 sibling, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2026-06-21 20:24 UTC (permalink / raw)
To: Jeff King; +Cc: D. Ben Knoble, Git
Jeff King <peff@peff.net> writes:
> I don't know if any of this is really worth digging too far. This feels
> like a case we could do a bit better at, but I wonder how much it
> matters in practice. As soon as you do any index-refresh (including "git
> status"), the racy entries are cleared and everything is faster. It
> just seems kind of lame that we write out the initial working tree with
> so many racy entries.
Yeah, We didn't want to stall for a full second back when we were
not using subsecond in anywhere, with nanosecond resolution
timestamps in place, we could delay writing the index file by 50
milliseconds, nobody notices the delay, and raciness would go away,
perhaps?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: git-diff in a worktree is an order of magnitude slower?
2026-06-21 20:24 ` Junio C Hamano
@ 2026-06-21 21:28 ` Jeff King
2026-06-21 23:17 ` Junio C Hamano
2026-06-22 12:20 ` Junio C Hamano
0 siblings, 2 replies; 17+ messages in thread
From: Jeff King @ 2026-06-21 21:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: D. Ben Knoble, Git
On Sun, Jun 21, 2026 at 01:24:53PM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I don't know if any of this is really worth digging too far. This feels
> > like a case we could do a bit better at, but I wonder how much it
> > matters in practice. As soon as you do any index-refresh (including "git
> > status"), the racy entries are cleared and everything is faster. It
> > just seems kind of lame that we write out the initial working tree with
> > so many racy entries.
>
> Yeah, We didn't want to stall for a full second back when we were
> not using subsecond in anywhere, with nanosecond resolution
> timestamps in place, we could delay writing the index file by 50
> milliseconds, nobody notices the delay, and raciness would go away,
> perhaps?
Yes, though that implies comparing the index and file mtimes with
nanosecond precision. We have that precision stored (at least
when the system supports it) but I'm not sure if that comparison would
run afoul of the reasons USE_NSEC was not the default in the first
place.
I guess not? The problem there is that the nanosecond portion would
sometimes get wiped if the entry was dropped from the kernel's in-memory
cache. And then stat-matching would not work. But if we are talking
about strictly asking "is this mtime later than that mtime", then I
think the worst case is that we fall back to the current behavior.
But at the point that we are comparing nanoseconds, I don't think we
even need to bother with the delay. It takes maybe 5 seconds to write
out all of the linux.git files and then the final index. So ~20% of
those files will have the same timestamp as the index. With nanosecond
resolution, we'd expect that to drop by an order of a billion. Even if
we get unlucky and have a single file with the same timestamp, that is
not so bad.
The code to do the nanosecond compare is already there! But it's gated
on USE_NSEC. So this (plus a bonus debugging trace ;) ):
diff --git a/read-cache.c b/read-cache.c
index 21829102ae..f84159a060 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -356,14 +356,10 @@ static int is_racy_stat(const struct index_state *istate,
const struct stat_data *sd)
{
return (istate->timestamp.sec &&
-#ifdef USE_NSEC
/* nanosecond timestamped files can also be racy! */
(istate->timestamp.sec < sd->sd_mtime.sec ||
(istate->timestamp.sec == sd->sd_mtime.sec &&
istate->timestamp.nsec <= sd->sd_mtime.nsec))
-#else
- istate->timestamp.sec <= sd->sd_mtime.sec
-#endif
);
}
@@ -434,6 +430,7 @@ int ie_match_stat(struct index_state *istate,
* carefully than others.
*/
if (!changed && is_racy_timestamp(istate, ce)) {
+ warning("%s is racy", ce->name);
if (assume_racy_is_modified)
changed |= DATA_CHANGED;
else
makes the problem go away. I'm not sure if I'm missing some case where
we could be bitten by the problem that led to making USE_NSEC
conditional, though.
-Peff
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: git-diff in a worktree is an order of magnitude slower?
2026-06-21 17:45 ` Jeff King
2026-06-21 20:24 ` Junio C Hamano
@ 2026-06-21 21:39 ` Junio C Hamano
2026-06-21 22:00 ` Jeff King
1 sibling, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2026-06-21 21:39 UTC (permalink / raw)
To: Jeff King; +Cc: D. Ben Knoble, Git
Jeff King <peff@peff.net> writes:
> BTW, I don't think diffcore actually has the information it would need
> to do so. The racy stuff is handled under the hood in ie_match_stat(),
> which returns only a set of "changed" flags. So the caller cannot tell
> the difference between the two cases:
>
> 1. We checked ce_match_stat_basic() which said "no change", and then
> is_racy_timestamp() was false, so that was good enough.
>
> 2. is_racy_timestamp() is true, so we further did a content check,
> found nothing, and returned the same "no change"
>
> Obviously we could pass back another flag, but that would disrupt the
> other callers. Hmm. It looks like we could pass in a flag to say "assume
> racy entries are modified". And then they come back to the diff code,
> diffcore_skip_stat_unmatch() sees they're not real diffs and suppresses
> them, but we _do_ count them as stat-dirty.
Yeah. Because ie_match_stat() does have access to istate, we could
add a new member to istate, next to "updated_workdir" and friends,
and smudge the bit when the is_racy_timestamp() goes to the
compare-data codepath and finds that we are better off auto
refreshing. Then "were we told to do skip-stat-unmatch and actually
found some that is worth refreshing?" code can be taught to pay
attention to that bit as well.
This is a tangent, but why do we call refresh_index_quietly() in the
central code path in cmd_diff() in the first place, I have to
wonder? It should not matter when we are comparing two tree objects
(or two commits), at least. It of course is not hurting, though.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: git-diff in a worktree is an order of magnitude slower?
2026-06-21 21:39 ` Junio C Hamano
@ 2026-06-21 22:00 ` Jeff King
0 siblings, 0 replies; 17+ messages in thread
From: Jeff King @ 2026-06-21 22:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: D. Ben Knoble, Git
On Sun, Jun 21, 2026 at 02:39:18PM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > BTW, I don't think diffcore actually has the information it would need
> > to do so. The racy stuff is handled under the hood in ie_match_stat(),
> > which returns only a set of "changed" flags. So the caller cannot tell
> > the difference between the two cases:
> >
> > 1. We checked ce_match_stat_basic() which said "no change", and then
> > is_racy_timestamp() was false, so that was good enough.
> >
> > 2. is_racy_timestamp() is true, so we further did a content check,
> > found nothing, and returned the same "no change"
> >
> > Obviously we could pass back another flag, but that would disrupt the
> > other callers. Hmm. It looks like we could pass in a flag to say "assume
> > racy entries are modified". And then they come back to the diff code,
> > diffcore_skip_stat_unmatch() sees they're not real diffs and suppresses
> > them, but we _do_ count them as stat-dirty.
>
> Yeah. Because ie_match_stat() does have access to istate, we could
> add a new member to istate, next to "updated_workdir" and friends,
> and smudge the bit when the is_racy_timestamp() goes to the
> compare-data codepath and finds that we are better off auto
> refreshing. Then "were we told to do skip-stat-unmatch and actually
> found some that is worth refreshing?" code can be taught to pay
> attention to that bit as well.
Yeah, that sounds fairly clean. Though if using nanoseconds works out
and makes racy entries extremely unlikely, that is better still. :)
> This is a tangent, but why do we call refresh_index_quietly() in the
> central code path in cmd_diff() in the first place, I have to
> wonder? It should not matter when we are comparing two tree objects
> (or two commits), at least. It of course is not hurting, though.
It seems like it could probably just go into builtin_diff_files(), but
are there other paths that might hit stat-unmatch entries? Maybe the
builtin_diff_b_f() path?
It probably should also support --no-optional-locks, which is currently
only respected by git-status. I don't think it matters that much in
practice because the point is reducing conflict with commands running
frequently in the background, and people don't tend to do that with
git-diff.
Back when we added --no-optional-locks, the idea was that people could
apply it in more spots if they ran into them in practice. So I guess
nobody has with git-diff.
-Peff
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: git-diff in a worktree is an order of magnitude slower?
2026-06-21 21:28 ` Jeff King
@ 2026-06-21 23:17 ` Junio C Hamano
2026-06-22 12:20 ` Junio C Hamano
1 sibling, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2026-06-21 23:17 UTC (permalink / raw)
To: Jeff King; +Cc: D. Ben Knoble, Git
Jeff King <peff@peff.net> writes:
> But at the point that we are comparing nanoseconds, I don't think we
> even need to bother with the delay. It takes maybe 5 seconds to write
> out all of the linux.git files and then the final index. So ~20% of
> those files will have the same timestamp as the index. With nanosecond
> resolution, we'd expect that to drop by an order of a billion. Even if
> we get unlucky and have a single file with the same timestamp, that is
> not so bad.
>
> The code to do the nanosecond compare is already there! But it's gated
> on USE_NSEC. So this (plus a bonus debugging trace ;) ):
> ...
> if (!changed && is_racy_timestamp(istate, ce)) {
> + warning("%s is racy", ce->name);
> if (assume_racy_is_modified)
> changed |= DATA_CHANGED;
> else
>...
> makes the problem go away. I'm not sure if I'm missing some case where
> we could be bitten by the problem that led to making USE_NSEC
> conditional, though.
That's cute.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: git-diff in a worktree is an order of magnitude slower?
2026-06-21 21:28 ` Jeff King
2026-06-21 23:17 ` Junio C Hamano
@ 2026-06-22 12:20 ` Junio C Hamano
1 sibling, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2026-06-22 12:20 UTC (permalink / raw)
To: Jeff King; +Cc: D. Ben Knoble, Git
Jeff King <peff@peff.net> writes:
> Yes, though that implies comparing the index and file mtimes with
> nanosecond precision. We have that precision stored (at least
> when the system supports it) but I'm not sure if that comparison would
> run afoul of the reasons USE_NSEC was not the default in the first
> place.
>
> I guess not? The problem there is that the nanosecond portion would
> sometimes get wiped if the entry was dropped from the kernel's in-memory
> cache. And then stat-matching would not work. But if we are talking
> about strictly asking "is this mtime later than that mtime", then I
> think the worst case is that we fall back to the current behavior.
Right, and you are right to point out that for the purpose of
comparing mtimes of files' and the index file, this would make it
unworkable. I can imagine that a file and the index may have been
written within the same millisecond but we can tell that the former
slightly earlier than the latter (or the other way around) with
nanoseconds resolution, then only one of the two lose the sub millisecond
resolution but not the other due to its in-core inode evicted out of
the cache. Depending on which one survives (and keeps a non-zero
sub millisecond part), they can compare differently.
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2026-06-22 12:20 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-08 23:36 git-diff in a worktree is an order of magnitude slower? D. Ben Knoble
2026-06-09 0:11 ` Jeff King
2026-06-09 17:15 ` D. Ben Knoble
2026-06-11 8:55 ` Jeff King
2026-06-11 17:43 ` Junio C Hamano
2026-06-11 21:06 ` brian m. carlson
2026-06-20 15:57 ` D. Ben Knoble
2026-06-21 0:53 ` Junio C Hamano
2026-06-21 3:58 ` Junio C Hamano
2026-06-21 17:24 ` Jeff King
2026-06-21 17:45 ` Jeff King
2026-06-21 20:24 ` Junio C Hamano
2026-06-21 21:28 ` Jeff King
2026-06-21 23:17 ` Junio C Hamano
2026-06-22 12:20 ` Junio C Hamano
2026-06-21 21:39 ` Junio C Hamano
2026-06-21 22:00 ` Jeff King
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox