{"thread":{"id":"65772","subject":"git-diff in a worktree is an order of magnitude slower?","startedAt":"2026-06-08T23:36:56Z","lastAt":"2026-07-03T15:57:33Z","messageCount":19,"participants":["D. Ben Knoble","Jeff King","Junio C Hamano","brian m. carlson"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"544981","messageId":"CALnO6CADMJSixqYvL1Yo8qKX5rWhKQ+2OoSEuPUh-yoeK9TseQ@mail.gmail.com","threadId":"65772","inReplyTo":null,"subject":"git-diff in a worktree is an order of magnitude slower?","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2026-06-08T23:36:45Z","receivedAt":"2026-06-08T23:36:56Z","isPatch":false,"body":"Hello all,\n\nI'd like to report and offer to help fix what I view as a serious performance\nbug:\n\n    \"git diff --no-ext-diff --quiet\" performs about ~10x slower in a secondary\n    worktree than in the main worktree.\n\nFortunately, this doesn't seem to extend to \"--cached\" (and \"--no-ext-diff\" and\n\"--quiet\" are probably both red-herrings, since it _does_ extend to plain \"git\ndiff\").\n\nHere's a short demo in Git:\n\n    # git switch -d v2.54.0\n    # ninja -C build # where my meson-generated build dir is\n    # git worktree add --detach ../perf-test v2.54.0\n    # hyperfine -N --warmup 10 './build/bin-wrappers/git diff'\n    Benchmark 1: ./build/bin-wrappers/git diff\n      Time (mean ± σ):       3.4 ms ±   0.5 ms    [User: 4.2 ms, System: 3.9 ms]\n      Range (min … max):     2.5 ms …   5.8 ms    677 runs\n    # pushd ../perf-test\n    # hyperfine -N --warmup 10 '../git/build/bin-wrappers/git diff'\n    Benchmark 1: ../git/build/bin-wrappers/git diff\n      Time (mean ± σ):     223.3 ms ±  10.5 ms    [User: 210.4 ms,\nSystem: 19.1 ms]\n      Range (min … max):   213.5 ms … 243.9 ms    13 runs\n\nI've had a similar experience at $DAYJOB, where a large repo takes ~6ms for the\nformer and ~650ms for the latter. I noticed because the Bash prompt functions\nexecute \"git diff --no-ext-diff --quiet\", and that was (AFAICT) the largest\nculprit for a slow shell prompt in a worktree. To squelch that from the prompt,\nI have to go down the rabbit hole of the worktree config extension, so I figured\nbetter to fix the slow diff if possible anyway.\n\n2 questions:\n\n1. Is this known, and if so is anybody working on it?\n2. How can I help identify problem areas?\n\nA little more\n-------------\n\nI've reproduced this as far back as v2.50.0, which is as far back as I could get\nthe meson build to work with little effort (so I can't rule out that this is an\nold regression).\n\nUsing \"perf record -F 99 -g -- <bin-wrappers/git> diff\" in both trees and then\n\"perf report\":\n\n- it looks like the main worktree spends most of it's time in preload_thread,\n  threaded_has_symlink_leading_path, lstat_cache…\n- the worktree spends a lot more time in ie_match_stat, ce_modified_check_fs,\n  ce_compare_data, index_fd, would_convert_to_git_filter_fd…\n\nHere's the relevant \"perf stat\":\n\nmain tree:\n\n Performance counter stats for './build/bin-wrappers/git diff':\n\n                 0      context-switches:u               #      0,0\ncs/sec  cs_per_second\n                 0      cpu-migrations:u                 #      0,0\nmigrations/sec  migrations_per_second\n               967      page-faults:u                    #  65036,4\nfaults/sec  page_faults_per_second\n             14,87 msec task-clock:u                     #      0,3\nCPUs  CPUs_utilized\n            48 616      branch-misses:u                  #      3,2 %\nbranch_miss_rate         (57,19%)\n         3 571 630      branches:u                       #    240,2\nM/sec  branch_frequency\n        13 635 411      cpu-cycles:u                     #      0,9\nGHz  cycles_frequency\n        22 120 068      instructions:u                   #      1,9\ninstructions  insn_per_cycle  (85,61%)\n         3 634 065      stalled-cycles-frontend:u        #     0,28\nfrontend_cycles_idle        (9,56%)\n\n       0,006860098 seconds time elapsed\n\n       0,001364000 seconds user\n       0,015157000 seconds sys\n\nworktree:\n\n Performance counter stats for '../git/build/bin-wrappers/git diff':\n\n                 0      context-switches:u               #      0,0\ncs/sec  cs_per_second\n                 0      cpu-migrations:u                 #      0,0\nmigrations/sec  migrations_per_second\n             1 585      page-faults:u                    #   5058,0\nfaults/sec  page_faults_per_second\n            313,37 msec task-clock:u                     #      0,9\nCPUs  CPUs_utilized\n         2 481 188      branch-misses:u                  #      1,5 %\nbranch_miss_rate         (48,94%)\n       168 664 155      branches:u                       #    538,2\nM/sec  branch_frequency     (51,21%)\n     1 004 095 217      cpu-cycles:u                     #      3,2\nGHz  cycles_frequency       (67,74%)\n     3 864 851 223      instructions:u                   #      3,9\ninstructions  insn_per_cycle  (52,73%)\n        70 755 234      stalled-cycles-frontend:u        #     0,07\nfrontend_cycles_idle        (49,29%)\n\n       0,306707634 seconds time elapsed\n\n       0,269027000 seconds user\n       0,045512000 seconds sys\n\nMy observations:\n- the worktree has ~twice as many page faults and\n- executes ~150 times as many instructions (3.8b compared to 23m).\n\n(When I try to run some \"perf\" stats as root to access other counters, like\nsyscalls, \"git diff\" in the worktree says \"not a git repository\", so I'm not\ncounting the actual behavior. Ditto with DTrace.)\n\nPS I almost CC'd Peff and Patrick, whose names stood out in \"git\nshortlog builtin/{worktree,diff}* object-file* | sort -t\\( -k2 -g\",\nbut decided they'd be their own best judge of whether they can\nunderstand what's going on? :)\n\n-- \nD. Ben Knoble\n"},{"id":"544988","messageId":"20260609001134.GD358144@coredump.intra.peff.net","threadId":"65772","inReplyTo":"CALnO6CADMJSixqYvL1Yo8qKX5rWhKQ+2OoSEuPUh-yoeK9TseQ@mail.gmail.com","subject":"Re: git-diff in a worktree is an order of magnitude slower?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-06-09T00:11:34Z","receivedAt":"2026-06-09T00:11:35Z","isPatch":false,"body":"On Mon, Jun 08, 2026 at 07:36:45PM -0400, D. Ben Knoble wrote:\n\n> I'd like to report and offer to help fix what I view as a serious performance\n> bug:\n> \n>     \"git diff --no-ext-diff --quiet\" performs about ~10x slower in a secondary\n>     worktree than in the main worktree.\n\nHmm, I get the opposite effect: it is much faster in the worktree!\n\nI did:\n\n  git clone /path/to/linux.git\n  git -C linux worktree add --detach ../wt\n  hyperfine -L dir linux,wt 'git -C {dir} diff'\n\nwhich yielded:\n\n  Benchmark 1: git -C linux diff\n    Time (mean ± σ):     188.9 ms ±   2.5 ms    [User: 166.4 ms, System: 130.7 ms]\n    Range (min … max):   185.5 ms … 194.8 ms    16 runs\n  \n  Benchmark 2: git -C wt diff\n    Time (mean ± σ):      20.0 ms ±   1.5 ms    [User: 23.4 ms, System: 103.5 ms]\n    Range (min … max):    17.2 ms …  24.6 ms    132 runs\n  \n  Summary\n    git -C wt diff ran\n      9.43 ± 0.71 times faster than git -C linux diff\n\nRunning:\n\n  perf record -g git -C wt --no-pager diff\n  perf record -g git -C linux --no-pager diff\n  perf diff\n\nimplies that the slow case is spending a lot more time computing sha1s.\nWhich implies that the entries are stat dirty. And indeed, if I run:\n\n  git -C linux update-index --refresh\n\nnow they both take ~20ms.\n\nI wonder if it's just a racy-git problem? Many files are written in the\nsame second as the index, so they end up with the same mtimes, and we\nhave to err on the side of checking the contents.\n\nSee Documentation/technical/racy-git.adoc for a larger discussion.\n\nSo it is not really about worktrees at all, but just \"bad luck\" in\ngenerating that initial index (that goes away next time you actually\nmake an index update that rewrites the whole thing).\n\nI'd have thought USE_NSEC was the default these days, but looks like it\nisn't? Try building with that and I'll bet it goes away entirely.\n\n> PS I almost CC'd Peff and Patrick, whose names stood out in \"git\n> shortlog builtin/{worktree,diff}* object-file* | sort -t\\( -k2 -g\",\n> but decided they'd be their own best judge of whether they can\n> understand what's going on? :)\n\nYou might be interested in \"git shortlog -ns\". :)\n\n-Peff\n"},{"id":"545083","messageId":"CALnO6CD+3sE1xQUnRsCFfWrZTsq2Edw7BWseLzasgT3dgtaq_Q@mail.gmail.com","threadId":"65772","inReplyTo":"20260609001134.GD358144@coredump.intra.peff.net","subject":"Re: git-diff in a worktree is an order of magnitude slower?","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2026-06-09T17:15:11Z","receivedAt":"2026-06-09T17:15:23Z","isPatch":false,"body":"On Mon, Jun 8, 2026 at 8:11 PM Jeff King <peff@peff.net> wrote:\n>\n> On Mon, Jun 08, 2026 at 07:36:45PM -0400, D. Ben Knoble wrote:\n>\n> > I'd like to report and offer to help fix what I view as a serious performance\n> > bug:\n> >\n> >     \"git diff --no-ext-diff --quiet\" performs about ~10x slower in a secondary\n> >     worktree than in the main worktree.\n>\n> Hmm, I get the opposite effect: it is much faster in the worktree!\n>\n> I did:\n>\n>   git clone /path/to/linux.git\n>   git -C linux worktree add --detach ../wt\n>   hyperfine -L dir linux,wt 'git -C {dir} diff'\n>\n> which yielded:\n>\n>   Benchmark 1: git -C linux diff\n>     Time (mean ± σ):     188.9 ms ±   2.5 ms    [User: 166.4 ms, System: 130.7 ms]\n>     Range (min … max):   185.5 ms … 194.8 ms    16 runs\n>\n>   Benchmark 2: git -C wt diff\n>     Time (mean ± σ):      20.0 ms ±   1.5 ms    [User: 23.4 ms, System: 103.5 ms]\n>     Range (min … max):    17.2 ms …  24.6 ms    132 runs\n>\n>   Summary\n>     git -C wt diff ran\n>       9.43 ± 0.71 times faster than git -C linux diff\n>\n> Running:\n>\n>   perf record -g git -C wt --no-pager diff\n>   perf record -g git -C linux --no-pager diff\n>   perf diff\n>\n> implies that the slow case is spending a lot more time computing sha1s.\n> Which implies that the entries are stat dirty. And indeed, if I run:\n>\n>   git -C linux update-index --refresh\n>\n> now they both take ~20ms.\n\nAh, TIL about --refresh. I suppose it could be nice if \"git diff\"\nupdated the index in this way, but that sounds like a band-aid. Maybe\ncreating a fresh worktree should do the equivalent to make sure it's\nconsidered \"fresh\"?\n\n(This also dropped my timings down to normal.)\n\nAt $DAYJOB, I _think_ some version of \"git restore <stuff>\" ended up\nalso updating the index.\n\n> I wonder if it's just a racy-git problem? Many files are written in the\n> same second as the index, so they end up with the same mtimes, and we\n> have to err on the side of checking the contents.\n>\n> See Documentation/technical/racy-git.adoc for a larger discussion.\n>\n> So it is not really about worktrees at all, but just \"bad luck\" in\n> generating that initial index (that goes away next time you actually\n> make an index update that rewrites the whole thing).\n\nAh, that makes sense! I'm familiar with the raciness but didn't expect it here.\n\n> I'd have thought USE_NSEC was the default these days, but looks like it\n> isn't? Try building with that and I'll bet it goes away entirely.\n\nThanks, I'll take a look.\n\nI can see on my Macbook that at least Meson does automatically set\neither USE_ST_TIMESPEC or NO_NSEC automatically, but has no option to\nenabled USE_NSEC and try that. I can probably write that patch (which\nI'll do to test), and I can send it along with the \"worktree add\nshould refresh the index\" if you think that's an appropriate thing to\ndo.\n\n> > PS I almost CC'd Peff and Patrick, whose names stood out in \"git\n> > shortlog builtin/{worktree,diff}* object-file* | sort -t\\( -k2 -g\",\n> > but decided they'd be their own best judge of whether they can\n> > understand what's going on? :)\n>\n> You might be interested in \"git shortlog -ns\". :)\n>\n> -Peff\n\nPhew! Yeah, that's much nicer. Thanks! (When typing this out for\nemail, I even left out the \"grep '^[^[:space:]]' |\" filter :P)\n\n-- \nD. Ben Knoble\n"},{"id":"545258","messageId":"20260611085526.GL2191159@coredump.intra.peff.net","threadId":"65772","inReplyTo":"CALnO6CD+3sE1xQUnRsCFfWrZTsq2Edw7BWseLzasgT3dgtaq_Q@mail.gmail.com","subject":"Re: git-diff in a worktree is an order of magnitude slower?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-06-11T08:55:26Z","receivedAt":"2026-06-11T08:55:28Z","isPatch":false,"body":"On Tue, Jun 09, 2026 at 01:15:11PM -0400, D. Ben Knoble wrote:\n\n> > Which implies that the entries are stat dirty. And indeed, if I run:\n> >\n> >   git -C linux update-index --refresh\n> >\n> > now they both take ~20ms.\n> \n> Ah, TIL about --refresh. I suppose it could be nice if \"git diff\"\n> updated the index in this way, but that sounds like a band-aid. Maybe\n> creating a fresh worktree should do the equivalent to make sure it's\n> considered \"fresh\"?\n\nI think \"git diff\" _does_ refresh the index internally (that's what\ntakes so long!). I thought we then wrote out the result, but maybe we\ndon't notice that it needs an update for some reason?\n\nI'm pretty sure \"git status\" does something similar, though running it\nin a slow working tree _does_ seem to make things faster. Maybe it's\nmore aggressive about doing the update.\n\nI don't think that refreshing after making a worktree would help. The\nproblem is one of timestamps: we just wrote an index (so it _should_ be\ntotally up to date), but we err on the side of caution for some entries\nbecause the file timestamps and the index timestamp are the same. So\nwhat makes it \"work\" is that one second passed between writing those\nfiles and running \"update-index\". If you ran it from the worktree\ncommand automatically, it might all still happen in the same second.\n\nAnd of course, it's not just worktrees. Any time we checkout we may\nsuffer from this problem, though initial clones and worktree creation\nwill write more files than most.\n\n\n> At $DAYJOB, I _think_ some version of \"git restore <stuff>\" ended up\n> also updating the index.\n\nYep, that would make sense. Any index write (after the second-hand\nticks) will make it go away, since it means updating the mtime of the\nindex.\n\n> > I'd have thought USE_NSEC was the default these days, but looks like it\n> > isn't? Try building with that and I'll bet it goes away entirely.\n> \n> Thanks, I'll take a look.\n> \n> I can see on my Macbook that at least Meson does automatically set\n> either USE_ST_TIMESPEC or NO_NSEC automatically, but has no option to\n> enabled USE_NSEC and try that. I can probably write that patch (which\n> I'll do to test), and I can send it along with the \"worktree add\n> should refresh the index\" if you think that's an appropriate thing to\n> do.\n\nI think NO_NSEC is about not looking at the nsec fields of stat structs\n(since they might not exist). But we don't actually use them for stat\nmatching unless USE_NSEC is set.\n\nI guess the distinction goes back to c06ff4908b (Record ns-timestamps if\npossible, but do not use it without USE_NSEC, 2009-03-04), which details\nsome reasons you might not want USE_NSEC. Feels like it ought to be a\nrun-time config, though, and maybe even something that gets auto-probed\nby git-init.\n\nDefinitely not an area I have looked at much, though, nor thought hard\nabout. So there might be gotchas. :)\n\n-Peff\n"},{"id":"545307","messageId":"xmqqbjdhnfaf.fsf@gitster.g","threadId":"65772","inReplyTo":"20260611085526.GL2191159@coredump.intra.peff.net","subject":"Re: git-diff in a worktree is an order of magnitude slower?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-06-11T17:43:52Z","receivedAt":"2026-06-11T17:43:54Z","isPatch":false,"body":"Jeff King <peff@peff.net> writes:\n\n> I guess the distinction goes back to c06ff4908b (Record ns-timestamps if\n> possible, but do not use it without USE_NSEC, 2009-03-04), which details\n> some reasons you might not want USE_NSEC. Feels like it ought to be a\n> run-time config, though, and maybe even something that gets auto-probed\n> by git-init.\n\nI thought for a bit but didn't think of a clean way to auto-probe if\na filesystem loses nanosecond-precision part of .st_Xtime when\n\"metadata is flushed and later read back in\" with reasonable\noverhead.  I do not think we want to trigger system-wide sync and/or\ndropping of buffer cache ;-)\n\n> Definitely not an area I have looked at much, though, nor thought hard\n> about. So there might be gotchas. :)\n>\n> -Peff\n"},{"id":"545316","messageId":"aisjSH1N2IWdhrtn@fruit.crustytoothpaste.net","threadId":"65772","inReplyTo":"xmqqbjdhnfaf.fsf@gitster.g","subject":"Re: git-diff in a worktree is an order of magnitude slower?","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2026-06-11T21:06:16Z","receivedAt":"2026-06-11T21:06:23Z","isPatch":false,"body":"On 2026-06-11 at 17:43:52, Junio C Hamano wrote:\n> Jeff King <peff@peff.net> writes:\n> \n> > I guess the distinction goes back to c06ff4908b (Record ns-timestamps if\n> > possible, but do not use it without USE_NSEC, 2009-03-04), which details\n> > some reasons you might not want USE_NSEC. Feels like it ought to be a\n> > run-time config, though, and maybe even something that gets auto-probed\n> > by git-init.\n> \n> I thought for a bit but didn't think of a clean way to auto-probe if\n> a filesystem loses nanosecond-precision part of .st_Xtime when\n> \"metadata is flushed and later read back in\" with reasonable\n> overhead.  I do not think we want to trigger system-wide sync and/or\n> dropping of buffer cache ;-)\n\nWe could have `git update-index` take options like it does for\n`--untracked-cache` and `--no-untracked-cache` to control these for\npeople who want them.  For instance, I know what operating system and\nfile system I'm using (Linux with btrfs), so if I know that option is\nsafe, I can enable it at runtime and reap the benefits.\n\nWe could even have `--test-use-nsec` to perform a `uname` and `statfs`\ncall to determine whether this is a known safe configuration if probing\nis not possible.\n-- \nbrian m. carlson (they/them)\nToronto, Ontario, CA\n"},{"id":"546036","messageId":"CALnO6CAx91kbJ84d6Ef655UNG0y0rhyknBRh6Y+0o7Xn-uVytQ@mail.gmail.com","threadId":"65772","inReplyTo":"20260611085526.GL2191159@coredump.intra.peff.net","subject":"Re: git-diff in a worktree is an order of magnitude slower?","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2026-06-20T15:57:29Z","receivedAt":"2026-06-20T15:57:40Z","isPatch":false,"body":"Coming back to index refreshing…\n\nOn Thu, Jun 11, 2026 at 4:55 AM Jeff King <peff@peff.net> wrote:\n>\n> On Tue, Jun 09, 2026 at 01:15:11PM -0400, D. Ben Knoble wrote:\n>\n> > > Which implies that the entries are stat dirty. And indeed, if I run:\n> > >\n> > >   git -C linux update-index --refresh\n> > >\n> > > now they both take ~20ms.\n> >\n> > Ah, TIL about --refresh. I suppose it could be nice if \"git diff\"\n> > updated the index in this way, but that sounds like a band-aid. Maybe\n> > creating a fresh worktree should do the equivalent to make sure it's\n> > considered \"fresh\"?\n>\n> I think \"git diff\" _does_ refresh the index internally (that's what\n> takes so long!). I thought we then wrote out the result, but maybe we\n> don't notice that it needs an update for some reason?\n>\n> I'm pretty sure \"git status\" does something similar, though running it\n> in a slow working tree _does_ seem to make things faster. Maybe it's\n> more aggressive about doing the update.\n\nThanks for the status pointer:\n\n- cmd_status calls refresh_index and repo_updated_index_if_able\n- those same calls are wrapped in refresh_index_quietly in builtin/diff.c\n\nBut the refresh_index_quietly call is guarded by (effectively; the\nactual code uses rev.diffopt.skip_stat_unmatch)\n\n    1 < !!diff_auto_refresh_index\n\nwhich dates to aecbf914c4 (git-diff: resurrect the traditional empty\n\"diff --git\" behaviour, 2007-08-31). On my system that comparison is\nfalse because the double-negation produces 1\n(diff_auto_refresh_index=1 or the result of git_config_bool). Or at\nleast, I don't see it get written to elsewhere (maybe that's supposed\nto happen in diff.c:diffcore_skip_stat_unmatch in this case and isn't?\nIdk. (Even dirtying the worktree as a hypothesis that only when a diff\nis found does the counter get bumped doesn't seem to work.)\n\nSo… has that conditional been quietly dead all this time? I can't\nimagine that's right, but…\n\n> > > I'd have thought USE_NSEC was the default these days, but looks like it\n> > > isn't? Try building with that and I'll bet it goes away entirely.\n> >\n> > Thanks, I'll take a look.\n> >\n> > I can see on my Macbook that at least Meson does automatically set\n> > either USE_ST_TIMESPEC or NO_NSEC automatically, but has no option to\n> > enabled USE_NSEC and try that. I can probably write that patch (which\n> > I'll do to test), and I can send it along with the \"worktree add\n> > should refresh the index\" if you think that's an appropriate thing to\n> > do.\n>\n> I think NO_NSEC is about not looking at the nsec fields of stat structs\n> (since they might not exist). But we don't actually use them for stat\n> matching unless USE_NSEC is set.\n>\n> I guess the distinction goes back to c06ff4908b (Record ns-timestamps if\n> possible, but do not use it without USE_NSEC, 2009-03-04), which details\n> some reasons you might not want USE_NSEC. Feels like it ought to be a\n> run-time config, though, and maybe even something that gets auto-probed\n> by git-init.\n>\n> Definitely not an area I have looked at much, though, nor thought hard\n> about. So there might be gotchas. :)\n>\n> -Peff\n\nLooks like adding USE_NSEC to my build did make the issue go away (the\npatch is short, and I'll send it anyway for folks to have the knob),\nbut that now seems like a band-aid to me based on my confusion above.\n\n-- \nD. Ben Knoble\n"},{"id":"546046","messageId":"xmqqa4sog1e9.fsf@gitster.g","threadId":"65772","inReplyTo":"CALnO6CAx91kbJ84d6Ef655UNG0y0rhyknBRh6Y+0o7Xn-uVytQ@mail.gmail.com","subject":"Re: git-diff in a worktree is an order of magnitude slower?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-06-21T00:53:02Z","receivedAt":"2026-06-21T00:53:05Z","isPatch":false,"body":"\"D. Ben Knoble\" <ben.knoble@gmail.com> writes:\n\n> But the refresh_index_quietly call is guarded by (effectively; the\n> actual code uses rev.diffopt.skip_stat_unmatch)\n>\n>     1 < !!diff_auto_refresh_index\n\nIt is not quite that, is it?  In aecbf914 (git-diff: resurrect the\ntraditional empty \"diff --git\" behaviour, 2007-08-31), it read more\nlike\n\n\tif (1 < rev.diffopt.skip_stat_unmatch)\n\t\trefresh_index_quietly();\n\nwhere rev.diffopt.skip_stat_unmatch was initialized to 1 if\ndiff_auto_refresh_index (boolean) is set to true.\n\nNow, cmd_diff() dispatches to various diff backends to compare two\nsets (like \"a tree object vs the index\", \"the index vs the working\ntree files\"), each of which ends with a call to diffcore_std() and\ndiffcore_flush() to conclude.   In diffcore_std() there is a call\nto diffcore_skip_stat_unmatch() ONLY when skip_stat_unmatch member\nis set (we initialize it to 1 when auto-refrresh-index is enabled,\nas you saw above).  The function is used to squelch the paths that\nremain in diff_queued_diff only because they were stat-dirty without\nhaving an actual content change, and _counts_ how many such ghost\nchanges existed by incrementing the .skip_stat_unmatch counter.\n\n> which dates to aecbf914c4 (git-diff: resurrect the traditional empty\n> \"diff --git\" behaviour, 2007-08-31). On my system that comparison is\n> false because the double-negation produces 1\n> (diff_auto_refresh_index=1 or the result of git_config_bool). \n\nNot quite.  It was false because double-negation initializes the\nmember to 1, which causes a call to diffcore_skip_stat_unmatch()\nbe made, *and* the diffcore_skip_stat_unmatch() function did not\nfind any ghost changes, i.e., paths that were only stat-dirty hence\nneeded a call to refresh_index_quietly().\n\n> So… has that conditional been quietly dead all this time? I can't\n> imagine that's right, but…\n\nI initially thought it was an embarrassing thinko, but after seeing\nhow .skip_stat_unmatch is used as a 1-based counter (i.e., if the\nmember says 42, it means it saw 41 paths that were stat-dirty but\nwithout actual content change), I do not think so.\n\nNow, it is a different matter if such a \"dual\" purpose \"more than a\nsimple boolean\" counter is a good idea.  Apparently it confused both\nof us in this case ;-).\n\nThanks.\n"},{"id":"546050","messageId":"xmqqse6gee9j.fsf@gitster.g","threadId":"65772","inReplyTo":"xmqqa4sog1e9.fsf@gitster.g","subject":"Re: git-diff in a worktree is an order of magnitude slower?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-06-21T03:58:00Z","receivedAt":"2026-06-21T03:58:03Z","isPatch":false,"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> I initially thought it was an embarrassing thinko, but after seeing\n> how .skip_stat_unmatch is used as a 1-based counter (i.e., if the\n> member says 42, it means it saw 41 paths that were stat-dirty but\n> without actual content change), I do not think so.\n>\n> Now, it is a different matter if such a \"dual\" purpose \"more than a\n> simple boolean\" counter is a good idea.  Apparently it confused both\n> of us in this case ;-).\n\nFWIW the patch was done as part of this discussion thread:\n\n  https://lore.kernel.org/git/20070830063810.GD16312@mellanox.co.il/\n\n"},{"id":"546080","messageId":"20260621172432.GA2206349@coredump.intra.peff.net","threadId":"65772","inReplyTo":"xmqqa4sog1e9.fsf@gitster.g","subject":"Re: git-diff in a worktree is an order of magnitude slower?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-06-21T17:24:32Z","receivedAt":"2026-06-21T17:24:41Z","isPatch":false,"body":"On Sat, Jun 20, 2026 at 05:53:02PM -0700, Junio C Hamano wrote:\n\n> > which dates to aecbf914c4 (git-diff: resurrect the traditional empty\n> > \"diff --git\" behaviour, 2007-08-31). On my system that comparison is\n> > false because the double-negation produces 1\n> > (diff_auto_refresh_index=1 or the result of git_config_bool). \n> \n> Not quite.  It was false because double-negation initializes the\n> member to 1, which causes a call to diffcore_skip_stat_unmatch()\n> be made, *and* the diffcore_skip_stat_unmatch() function did not\n> find any ghost changes, i.e., paths that were only stat-dirty hence\n> needed a call to refresh_index_quietly().\n\nI think this is the core of the issue. These entries are \"racy git\ndirty\" in the sense that their mtimes are the same as the index mtime,\nand so we double-check the contents. This is the first bullet point\nunder the \"Racy Git\" section of Documentation/technical/racy-git.adoc.\n\nBut diffcore_skip_stat_unmatch() doesn't count them as dirty, so we\ndon't increment the counter, and thus top-level git-diff won't write out\nthe new index. And thus every subsequent diff repeats the same\nexpensive double-check.\n\nBut I'm not sure where the blame lies. Either:\n\n  1. diffcore_skip_stat_unmatch() should be counting these in its\n     \"dirty\" counter; or\n\n  2. the index should be marking these differently. The second bullet\n     point of that Racy Git section says:\n\n       When the index file is updated that contains racily clean\n       entries, cached `st_size` information is truncated to zero\n       before writing a new version of the index file.\n\n     Should the index be written out with a 0 size field here, so that\n     we know they are dirty and should be updated? I guess that would be\n     user-visible, though, because commands that _don't_ update the\n     index (like plumbing diff-files) would generate a spurious diff\n     there rather than doing the content-level comparison.\n\nI dunno. You had solved most of the racy git stuff before I came along,\nso I never gave it too much thought (and what little thought I did was\nmany years ago).\n\n> > So… has that conditional been quietly dead all this time? I can't\n> > imagine that's right, but…\n> \n> I initially thought it was an embarrassing thinko, but after seeing\n> how .skip_stat_unmatch is used as a 1-based counter (i.e., if the\n> member says 42, it means it saw 41 paths that were stat-dirty but\n> without actual content change), I do not think so.\n> \n> Now, it is a different matter if such a \"dual\" purpose \"more than a\n> simple boolean\" counter is a good idea.  Apparently it confused both\n> of us in this case ;-).\n\nMake that three of us. ;)\n\n-Peff\n"},{"id":"546081","messageId":"20260621174518.GB2206349@coredump.intra.peff.net","threadId":"65772","inReplyTo":"20260621172432.GA2206349@coredump.intra.peff.net","subject":"Re: git-diff in a worktree is an order of magnitude slower?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-06-21T17:45:18Z","receivedAt":"2026-06-21T17:45:20Z","isPatch":false,"body":"On Sun, Jun 21, 2026 at 01:24:32PM -0400, Jeff King wrote:\n\n> I think this is the core of the issue. These entries are \"racy git\n> dirty\" in the sense that their mtimes are the same as the index mtime,\n> and so we double-check the contents. This is the first bullet point\n> under the \"Racy Git\" section of Documentation/technical/racy-git.adoc.\n> \n> But diffcore_skip_stat_unmatch() doesn't count them as dirty, so we\n> don't increment the counter, and thus top-level git-diff won't write out\n> the new index. And thus every subsequent diff repeats the same\n> expensive double-check.\n> \n> But I'm not sure where the blame lies. Either:\n> \n>   1. diffcore_skip_stat_unmatch() should be counting these in its\n>      \"dirty\" counter; or\n\nBTW, I don't think diffcore actually has the information it would need\nto do so. The racy stuff is handled under the hood in ie_match_stat(),\nwhich returns only a set of \"changed\" flags. So the caller cannot tell\nthe difference between the two cases:\n\n  1. We checked ce_match_stat_basic() which said \"no change\", and then\n     is_racy_timestamp() was false, so that was good enough.\n\n  2. is_racy_timestamp() is true, so we further did a content check,\n     found nothing, and returned the same \"no change\"\n\nObviously we could pass back another flag, but that would disrupt the\nother callers. Hmm. It looks like we could pass in a flag to say \"assume\nracy entries are modified\". And then they come back to the diff code,\ndiffcore_skip_stat_unmatch() sees they're not real diffs and suppresses\nthem, but we _do_ count them as stat-dirty.\n\nLike this:\n\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex 4b46e394ce..4d36b5c1e0 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -271,6 +271,9 @@ static void builtin_diff_files(struct rev_info *revs, int argc, const char **arg\n \t\targv++; argc--;\n \t}\n \n+\tif (revs->diffopt.skip_stat_unmatch)\n+\t\toptions |= DIFF_RACY_IS_MODIFIED;\n+\n \t/*\n \t * \"diff --base\" should not combine merges because it was not\n \t * asked to.  \"diff -c\" should not densify (if the user wants\n\nThat seems to work, in the sense that \"git diff\" does refresh the index\nafterwards. But the timings are a bit funny.\n\nIn my working tree of linux.git with many racy entries it was ~500ms to\ndo the first diff (and the second, and so on, because we never updated\nthe index). After the patch above, it is 1800ms to do the first diff,\nand then fast (~30ms) after.\n\nI could believe it takes twice as long when we refresh the index\n(because I don't think we use the stat-cleanliness we collected from the\ndiff, but rather just do a from-scratch index refresh). But that would\nimply it should take ~1000ms. Where does the extra 800ms go? I guess\nthat somehow the content-check done by diffcore_skip_stat_unmatch() is\nslower than the one done by ie_match_stat(). I think the individual\nfunctions are respectively diff_filespec_check_stat_unmatch() and\nce_modified_check_fs().\n\nI don't know if any of this is really worth digging too far. This feels\nlike a case we could do a bit better at, but I wonder how much it\nmatters in practice. As soon as you do any index-refresh (including \"git\nstatus\"), the racy entries are cleared and everything is faster. It\njust seems kind of lame that we write out the initial working tree with\nso many racy entries.\n\n-Peff\n"},{"id":"546087","messageId":"xmqqfr2f7iay.fsf@gitster.g","threadId":"65772","inReplyTo":"20260621174518.GB2206349@coredump.intra.peff.net","subject":"Re: git-diff in a worktree is an order of magnitude slower?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-06-21T20:24:53Z","receivedAt":"2026-06-21T20:24:56Z","isPatch":false,"body":"Jeff King <peff@peff.net> writes:\n\n> I don't know if any of this is really worth digging too far. This feels\n> like a case we could do a bit better at, but I wonder how much it\n> matters in practice. As soon as you do any index-refresh (including \"git\n> status\"), the racy entries are cleared and everything is faster. It\n> just seems kind of lame that we write out the initial working tree with\n> so many racy entries.\n\nYeah, We didn't want to stall for a full second back when we were\nnot using subsecond in anywhere, with nanosecond resolution\ntimestamps in place, we could delay writing the index file by 50\nmilliseconds, nobody notices the delay, and raciness would go away,\nperhaps?\n"},{"id":"546091","messageId":"20260621212805.GB2297179@coredump.intra.peff.net","threadId":"65772","inReplyTo":"xmqqfr2f7iay.fsf@gitster.g","subject":"Re: git-diff in a worktree is an order of magnitude slower?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-06-21T21:28:05Z","receivedAt":"2026-06-21T21:28:06Z","isPatch":false,"body":"On Sun, Jun 21, 2026 at 01:24:53PM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > I don't know if any of this is really worth digging too far. This feels\n> > like a case we could do a bit better at, but I wonder how much it\n> > matters in practice. As soon as you do any index-refresh (including \"git\n> > status\"), the racy entries are cleared and everything is faster. It\n> > just seems kind of lame that we write out the initial working tree with\n> > so many racy entries.\n> \n> Yeah, We didn't want to stall for a full second back when we were\n> not using subsecond in anywhere, with nanosecond resolution\n> timestamps in place, we could delay writing the index file by 50\n> milliseconds, nobody notices the delay, and raciness would go away,\n> perhaps?\n\nYes, though that implies comparing the index and file mtimes with\nnanosecond precision.  We have that precision stored (at least\nwhen the system supports it) but I'm not sure if that comparison would\nrun afoul of the reasons USE_NSEC was not the default in the first\nplace.\n\nI guess not? The problem there is that the nanosecond portion would\nsometimes get wiped if the entry was dropped from the kernel's in-memory\ncache. And then stat-matching would not work. But if we are talking\nabout strictly asking \"is this mtime later than that mtime\", then I\nthink the worst case is that we fall back to the current behavior.\n\nBut at the point that we are comparing nanoseconds, I don't think we\neven need to bother with the delay. It takes maybe 5 seconds to write\nout all of the linux.git files and then the final index. So ~20% of\nthose files will have the same timestamp as the index. With nanosecond\nresolution, we'd expect that to drop by an order of a billion. Even if\nwe get unlucky and have a single file with the same timestamp, that is\nnot so bad.\n\nThe code to do the nanosecond compare is already there! But it's gated\non USE_NSEC. So this (plus a bonus debugging trace ;) ):\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 21829102ae..f84159a060 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -356,14 +356,10 @@ static int is_racy_stat(const struct index_state *istate,\n \t\t\tconst struct stat_data *sd)\n {\n \treturn (istate->timestamp.sec &&\n-#ifdef USE_NSEC\n \t\t /* nanosecond timestamped files can also be racy! */\n \t\t(istate->timestamp.sec < sd->sd_mtime.sec ||\n \t\t (istate->timestamp.sec == sd->sd_mtime.sec &&\n \t\t  istate->timestamp.nsec <= sd->sd_mtime.nsec))\n-#else\n-\t\tistate->timestamp.sec <= sd->sd_mtime.sec\n-#endif\n \t\t);\n }\n \n@@ -434,6 +430,7 @@ int ie_match_stat(struct index_state *istate,\n \t * carefully than others.\n \t */\n \tif (!changed && is_racy_timestamp(istate, ce)) {\n+\t\twarning(\"%s is racy\", ce->name);\n \t\tif (assume_racy_is_modified)\n \t\t\tchanged |= DATA_CHANGED;\n \t\telse\n\nmakes the problem go away. I'm not sure if I'm missing some case where\nwe could be bitten by the problem that led to making USE_NSEC\nconditional, though.\n\n-Peff\n"},{"id":"546093","messageId":"xmqqechz60ah.fsf@gitster.g","threadId":"65772","inReplyTo":"20260621174518.GB2206349@coredump.intra.peff.net","subject":"Re: git-diff in a worktree is an order of magnitude slower?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-06-21T21:39:18Z","receivedAt":"2026-06-21T21:39:21Z","isPatch":false,"body":"Jeff King <peff@peff.net> writes:\n\n> BTW, I don't think diffcore actually has the information it would need\n> to do so. The racy stuff is handled under the hood in ie_match_stat(),\n> which returns only a set of \"changed\" flags. So the caller cannot tell\n> the difference between the two cases:\n>\n>   1. We checked ce_match_stat_basic() which said \"no change\", and then\n>      is_racy_timestamp() was false, so that was good enough.\n>\n>   2. is_racy_timestamp() is true, so we further did a content check,\n>      found nothing, and returned the same \"no change\"\n>\n> Obviously we could pass back another flag, but that would disrupt the\n> other callers. Hmm. It looks like we could pass in a flag to say \"assume\n> racy entries are modified\". And then they come back to the diff code,\n> diffcore_skip_stat_unmatch() sees they're not real diffs and suppresses\n> them, but we _do_ count them as stat-dirty.\n\nYeah.  Because ie_match_stat() does have access to istate, we could\nadd a new member to istate, next to \"updated_workdir\" and friends,\nand smudge the bit when the is_racy_timestamp() goes to the\ncompare-data codepath and finds that we are better off auto\nrefreshing.  Then \"were we told to do skip-stat-unmatch and actually\nfound some that is worth refreshing?\" code can be taught to pay\nattention to that bit as well.\n\nThis is a tangent, but why do we call refresh_index_quietly() in the\ncentral code path in cmd_diff() in the first place, I have to\nwonder?  It should not matter when we are comparing two tree objects\n(or two commits), at least.  It of course is not hurting, though.\n"},{"id":"546095","messageId":"20260621220046.GE2297179@coredump.intra.peff.net","threadId":"65772","inReplyTo":"xmqqechz60ah.fsf@gitster.g","subject":"Re: git-diff in a worktree is an order of magnitude slower?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-06-21T22:00:46Z","receivedAt":"2026-06-21T22:00:47Z","isPatch":false,"body":"On Sun, Jun 21, 2026 at 02:39:18PM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > BTW, I don't think diffcore actually has the information it would need\n> > to do so. The racy stuff is handled under the hood in ie_match_stat(),\n> > which returns only a set of \"changed\" flags. So the caller cannot tell\n> > the difference between the two cases:\n> >\n> >   1. We checked ce_match_stat_basic() which said \"no change\", and then\n> >      is_racy_timestamp() was false, so that was good enough.\n> >\n> >   2. is_racy_timestamp() is true, so we further did a content check,\n> >      found nothing, and returned the same \"no change\"\n> >\n> > Obviously we could pass back another flag, but that would disrupt the\n> > other callers. Hmm. It looks like we could pass in a flag to say \"assume\n> > racy entries are modified\". And then they come back to the diff code,\n> > diffcore_skip_stat_unmatch() sees they're not real diffs and suppresses\n> > them, but we _do_ count them as stat-dirty.\n> \n> Yeah.  Because ie_match_stat() does have access to istate, we could\n> add a new member to istate, next to \"updated_workdir\" and friends,\n> and smudge the bit when the is_racy_timestamp() goes to the\n> compare-data codepath and finds that we are better off auto\n> refreshing.  Then \"were we told to do skip-stat-unmatch and actually\n> found some that is worth refreshing?\" code can be taught to pay\n> attention to that bit as well.\n\nYeah, that sounds fairly clean. Though if using nanoseconds works out\nand makes racy entries extremely unlikely, that is better still. :)\n\n> This is a tangent, but why do we call refresh_index_quietly() in the\n> central code path in cmd_diff() in the first place, I have to\n> wonder?  It should not matter when we are comparing two tree objects\n> (or two commits), at least.  It of course is not hurting, though.\n\nIt seems like it could probably just go into builtin_diff_files(), but\nare there other paths that might hit stat-unmatch entries? Maybe the\nbuiltin_diff_b_f() path?\n\nIt probably should also support --no-optional-locks, which is currently\nonly respected by git-status. I don't think it matters that much in\npractice because the point is reducing conflict with commands running\nfrequently in the background, and people don't tend to do that with\ngit-diff.\n\nBack when we added --no-optional-locks, the idea was that people could\napply it in more spots if they ran into them in practice. So I guess\nnobody has with git-diff.\n\n-Peff\n"},{"id":"546101","messageId":"xmqqa4sn5vqg.fsf@gitster.g","threadId":"65772","inReplyTo":"20260621212805.GB2297179@coredump.intra.peff.net","subject":"Re: git-diff in a worktree is an order of magnitude slower?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-06-21T23:17:43Z","receivedAt":"2026-06-21T23:17:45Z","isPatch":false,"body":"Jeff King <peff@peff.net> writes:\n\n> But at the point that we are comparing nanoseconds, I don't think we\n> even need to bother with the delay. It takes maybe 5 seconds to write\n> out all of the linux.git files and then the final index. So ~20% of\n> those files will have the same timestamp as the index. With nanosecond\n> resolution, we'd expect that to drop by an order of a billion. Even if\n> we get unlucky and have a single file with the same timestamp, that is\n> not so bad.\n>\n> The code to do the nanosecond compare is already there! But it's gated\n> on USE_NSEC. So this (plus a bonus debugging trace ;) ):\n> ...\n>  \tif (!changed && is_racy_timestamp(istate, ce)) {\n> +\t\twarning(\"%s is racy\", ce->name);\n>  \t\tif (assume_racy_is_modified)\n>  \t\t\tchanged |= DATA_CHANGED;\n>  \t\telse\n>...\n> makes the problem go away. I'm not sure if I'm missing some case where\n> we could be bitten by the problem that led to making USE_NSEC\n> conditional, though.\n\nThat's cute.\n"},{"id":"546169","messageId":"xmqqik7a4vhp.fsf@gitster.g","threadId":"65772","inReplyTo":"20260621212805.GB2297179@coredump.intra.peff.net","subject":"Re: git-diff in a worktree is an order of magnitude slower?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-06-22T12:20:34Z","receivedAt":"2026-06-22T12:20:36Z","isPatch":false,"body":"Jeff King <peff@peff.net> writes:\n\n> Yes, though that implies comparing the index and file mtimes with\n> nanosecond precision.  We have that precision stored (at least\n> when the system supports it) but I'm not sure if that comparison would\n> run afoul of the reasons USE_NSEC was not the default in the first\n> place.\n>\n> I guess not? The problem there is that the nanosecond portion would\n> sometimes get wiped if the entry was dropped from the kernel's in-memory\n> cache. And then stat-matching would not work. But if we are talking\n> about strictly asking \"is this mtime later than that mtime\", then I\n> think the worst case is that we fall back to the current behavior.\n\nRight, and you are right to point out that for the purpose of\ncomparing mtimes of files' and the index file, this would make it\nunworkable.  I can imagine that a file and the index may have been\nwritten within the same millisecond but we can tell that the former\nslightly earlier than the latter (or the other way around) with\nnanoseconds resolution, then only one of the two lose the sub millisecond\nresolution but not the other due to its in-core inode evicted out of\nthe cache.  Depending on which one survives (and keeps a non-zero\nsub millisecond part), they can compare differently.\n"},{"id":"546593","messageId":"20260628083628.GB3594700@coredump.intra.peff.net","threadId":"65772","inReplyTo":"xmqqik7a4vhp.fsf@gitster.g","subject":"Re: git-diff in a worktree is an order of magnitude slower?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-06-28T08:36:28Z","receivedAt":"2026-06-28T08:36:30Z","isPatch":false,"body":"On Mon, Jun 22, 2026 at 05:20:34AM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > Yes, though that implies comparing the index and file mtimes with\n> > nanosecond precision.  We have that precision stored (at least\n> > when the system supports it) but I'm not sure if that comparison would\n> > run afoul of the reasons USE_NSEC was not the default in the first\n> > place.\n> >\n> > I guess not? The problem there is that the nanosecond portion would\n> > sometimes get wiped if the entry was dropped from the kernel's in-memory\n> > cache. And then stat-matching would not work. But if we are talking\n> > about strictly asking \"is this mtime later than that mtime\", then I\n> > think the worst case is that we fall back to the current behavior.\n> \n> Right, and you are right to point out that for the purpose of\n> comparing mtimes of files' and the index file, this would make it\n> unworkable.  I can imagine that a file and the index may have been\n> written within the same millisecond but we can tell that the former\n> slightly earlier than the latter (or the other way around) with\n> nanoseconds resolution, then only one of the two lose the sub millisecond\n> resolution but not the other due to its in-core inode evicted out of\n> the cache.  Depending on which one survives (and keeps a non-zero\n> sub millisecond part), they can compare differently.\n\nHmm, yeah. I was thinking there might be some mitigating factor because\nwe're comparing stat information that is stored in the index, and not\nagainst a fresh stat() call. But that's not true.\n\nWe are using stat() information stored in the index from a file that was\nwritten in the same second as that index (otherwise we do not care about\nnanoseconds at all). But the index does not store its own mtime. At the\ntime of reading, we will fstat() it fresh, so we may see the truncated\nmtime value.\n\nIn that case we'd always see the index as older than it really is. Which\nI think does fail in the favorable direction for us (we assume the\ntoo-new file is possibly racy and err on the side of caution).\n\nBut I don't think it rules out seeing the truncation in the other\ndirection. The original index write would have to be done in the same\nsecond that the tracked file is written (because we only care about\nnanoseconds when that is true). So it implies that the tracked file was\nwritten, had its inode evicted, and then was re-read from disk all in\nthe same second that the index is being written, and the index inode\nitself is never evicted. That seems unlikely but not impossible.\n\nAnyway, it's all sufficiently scary that I think it should stay\nconditional on USE_NSEC. I do suspect that USE_NSEC is safe at least on\nLinux these days (see my response to Patrick elsewhere).\n\n-Peff\n"},{"id":"547107","messageId":"CALnO6CBC+SK=ycHn4xgzoZAud5ZWpqSp1NSe5maKPhp9+f=LgQ@mail.gmail.com","threadId":"65772","inReplyTo":"20260621174518.GB2206349@coredump.intra.peff.net","subject":"Re: git-diff in a worktree is an order of magnitude slower?","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2026-07-03T15:57:21Z","receivedAt":"2026-07-03T15:57:33Z","isPatch":false,"body":"On Sun, Jun 21, 2026 at 1:45 PM Jeff King <peff@peff.net> wrote:\n>\n> On Sun, Jun 21, 2026 at 01:24:32PM -0400, Jeff King wrote:\n>\n> > I think this is the core of the issue. These entries are \"racy git\n> > dirty\" in the sense that their mtimes are the same as the index mtime,\n> > and so we double-check the contents. This is the first bullet point\n> > under the \"Racy Git\" section of Documentation/technical/racy-git.adoc.\n> >\n> > But diffcore_skip_stat_unmatch() doesn't count them as dirty, so we\n> > don't increment the counter, and thus top-level git-diff won't write out\n> > the new index. And thus every subsequent diff repeats the same\n> > expensive double-check.\n> >\n> > But I'm not sure where the blame lies. Either:\n> >\n> >   1. diffcore_skip_stat_unmatch() should be counting these in its\n> >      \"dirty\" counter; or\n>\n> BTW, I don't think diffcore actually has the information it would need\n> to do so. The racy stuff is handled under the hood in ie_match_stat(),\n> which returns only a set of \"changed\" flags. So the caller cannot tell\n> the difference between the two cases:\n>\n>   1. We checked ce_match_stat_basic() which said \"no change\", and then\n>      is_racy_timestamp() was false, so that was good enough.\n>\n>   2. is_racy_timestamp() is true, so we further did a content check,\n>      found nothing, and returned the same \"no change\"\n>\n> Obviously we could pass back another flag, but that would disrupt the\n> other callers. Hmm. It looks like we could pass in a flag to say \"assume\n> racy entries are modified\". And then they come back to the diff code,\n> diffcore_skip_stat_unmatch() sees they're not real diffs and suppresses\n> them, but we _do_ count them as stat-dirty.\n>\n> Like this:\n>\n> diff --git a/builtin/diff.c b/builtin/diff.c\n> index 4b46e394ce..4d36b5c1e0 100644\n> --- a/builtin/diff.c\n> +++ b/builtin/diff.c\n> @@ -271,6 +271,9 @@ static void builtin_diff_files(struct rev_info *revs, int argc, const char **arg\n>                 argv++; argc--;\n>         }\n>\n> +       if (revs->diffopt.skip_stat_unmatch)\n> +               options |= DIFF_RACY_IS_MODIFIED;\n> +\n>         /*\n>          * \"diff --base\" should not combine merges because it was not\n>          * asked to.  \"diff -c\" should not densify (if the user wants\n>\n> That seems to work, in the sense that \"git diff\" does refresh the index\n> afterwards. But the timings are a bit funny.\n>\n> In my working tree of linux.git with many racy entries it was ~500ms to\n> do the first diff (and the second, and so on, because we never updated\n> the index). After the patch above, it is 1800ms to do the first diff,\n> and then fast (~30ms) after.\n>\n> I could believe it takes twice as long when we refresh the index\n> (because I don't think we use the stat-cleanliness we collected from the\n> diff, but rather just do a from-scratch index refresh). But that would\n> imply it should take ~1000ms. Where does the extra 800ms go? I guess\n> that somehow the content-check done by diffcore_skip_stat_unmatch() is\n> slower than the one done by ie_match_stat(). I think the individual\n> functions are respectively diff_filespec_check_stat_unmatch() and\n> ce_modified_check_fs().\n>\n> I don't know if any of this is really worth digging too far. This feels\n> like a case we could do a bit better at, but I wonder how much it\n> matters in practice. As soon as you do any index-refresh (including \"git\n> status\"), the racy entries are cleared and everything is faster. It\n> just seems kind of lame that we write out the initial working tree with\n> so many racy entries.\n>\n> -Peff\n\nI'd like to dig into this some more, personally, but I'm not sure when\nI'll have the time (and we're deep in the guts of 2 systems whose\nimplementation are quite foreign to me---the index and the diff\nmachinery). The main reason is that I noticed this all when trying to\nfigure out why my shell prompt was slow :) I'm willing to pay a slow\nfirst prompt for all subsequent prompts to be faster without having to\nremember (and alert others) \"oh, this is racy git, just run 'git\nstatus' to fix it\" or something. Obviously it's even better if that\nfirst racy diff + index update is not so slow, though.\n\nI think I saw 2 potential areas to dig?\n1. The time spent on that refresh index diff mentioned above\n2. Limiting racy entries on initial write.\n\nThe latter was, I think, dismissed down-thread if I understood? It's\nnot so nice to stall for a full second just to avoid raciness, and\nUSE_NSEC alleviates the problem, too. (If that became available to\nmore folks, see USE_NSEC meson thread, then I suppose I would be less\nlikely to dig into (1) even though it sounds like an interesting\npuzzle.) So maybe instead of \"dismissed\" I mean \"we decided to keep\nthe USE_NSEC gate.\"\n\nThe former I saw some interesting discussion about how to communicate\nbits to code, but no hint as to whether that changed your initial\nmeasurements. I suppose I could try for myself, but it will take me\nsome time to process Junio's suggestions there into actual code.\n\n-- \nD. Ben Knoble\n"}]}