{"thread":{"id":"65777","subject":"[PATCH v2] ls-files: filter pathspec before lstat","startedAt":"2026-06-09T02:37:24Z","lastAt":"2026-06-15T15:27:32Z","messageCount":12,"participants":["Tamir Duberstein","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"545001","messageId":"20260608-ls-files-pathspec-lstat-v2-1-fb734b28422e@gmail.com","threadId":"65777","inReplyTo":null,"subject":"[PATCH v2] ls-files: filter pathspec before lstat","fromName":"Tamir Duberstein","fromEmail":"tamird@gmail.com","sentAt":"2026-06-09T02:37:15Z","receivedAt":"2026-06-09T02:37:24Z","isPatch":true,"body":"show_files() checks whether each index entry is deleted or modified\nbefore show_ce() applies the pathspec. prune_index() avoids most of this\nwork for pathspecs with a common directory prefix, but a top-level name\nor leading wildcard leaves every entry to be checked.\n\nFor a single pathspec, match it before lstat() in the deleted and\nmodified modes. Keep the later match in show_ce() so --error-unmatch is\nsatisfied only by entries that are actually shown.\n\nmatch_pathspec() is linear in the number of pathspec items. Applying it\nearly for every item can therefore multiply the work for commands with\nmany pathspecs, especially when lstat() shows that no entries are\nmodified. Restrict the early check to one pathspec. Callers with\nmultiple pathspecs retain the existing lstat()-first order.\n\nOn a repository with 859,211 index entries, a 19,931,862-byte index,\nand 25,303,439 packed objects occupying 21.13 GiB, I exported $parent\nand $this to binaries built from the parent and this commit, then ran:\n\n hyperfine --warmup 0 --runs 3 \\\n --command-name parent \\\n '$parent -c core.fsmonitor=false ls-files --deleted -- README.md' \\\n --command-name 'this commit' \\\n '$this -c core.fsmonitor=false ls-files --deleted -- README.md'\n\nThe results were:\n\n parent this commit\n elapsed 60.742 s 1.061 s\n user 1.117 s 0.963 s\n system 10.740 s 0.042 s\n\nFor an all-matching pathspec, I used a checkout with 859,940 index\nentries and ran:\n\n hyperfine --warmup 0 --runs 3 \\\n --command-name parent \\\n '$parent -c core.fsmonitor=false ls-files --deleted -- \"*\"' \\\n --command-name 'this commit' \\\n '$this -c core.fsmonitor=false ls-files --deleted -- \"*\"'\n\nI repeated the benchmark with the commands reversed. The results were:\n\n parent this commit\n parent first elapsed 56.807 s 64.618 s\n user 1.256 s 1.270 s\n system 10.633 s 11.068 s\n patched first elapsed 63.361 s 64.316 s\n user 1.238 s 1.280 s\n system 10.296 s 11.864 s\n\nThe patched user-time means were 14 ms and 42 ms higher in the two\norderings. Elapsed time changed by several seconds when the order was\nreversed, so those results do not show a stable wall-time ordering.\n\nJeff King pointed out that a preliminary match for each of many literal\npathspecs can be much more expensive. On a generated repository with\n10,000 clean files, I recorded the paths with \"git ls-files >paths\".\nWith $v1 exported to a binary built from the implementation sent in v1,\nI ran:\n\n hyperfine --warmup 2 --runs 10 \\\n --command-name parent \\\n '$parent ls-files -m -- $(cat paths) >/dev/null' \\\n --command-name 'this commit' \\\n '$this ls-files -m -- $(cat paths) >/dev/null'\n\nI replaced $this with $v1 in a second invocation. The wall-clock means\nand standard deviations were:\n\n mean standard deviation\n parent, final run 110.1 ms 4.1 ms\n this commit 104.9 ms 2.2 ms\n parent, v1 run 112.5 ms 6.6 ms\n unguarded v1 494.1 ms 17.2 ms\n\nThe guarded result matches the parent within the observed variation,\nwhile avoiding the regression in v1.\n\nAll three revisions were built with -O3, -mcpu=native, and ThinLTO\nusing Apple clang 21.0.0 on macOS 26.5. The machine was a MacBook Pro\n(Mac16,6) with a 16-core Apple M4 Max (12 performance and four\nefficiency cores) and 128 GB RAM.\n\nLink: https://lore.kernel.org/r/20260607-ls-files-pathspec-lstat-v1-1-8cf40b730146@gmail.com\nHelped-by: Jeff King \nSigned-off-by: Tamir Duberstein \n---\nA selective pathspec should let ls-files --deleted and --modified avoid\nstatting entries that cannot be shown. Match a single pathspec before\naccessing the worktree, while preserving the existing lstat-first order\nfor multiple pathspecs whose matching cost grows linearly.\n---\nChanges in v2:\n- Restrict early matching to one pathspec, avoiding the regression Jeff\n demonstrated with many pathspecs.\n- Add all-matching and many-pathspec performance results.\n- Drop the Assisted-by trailer.\n- Link to v1: https://patch.msgid.link/20260607-ls-files-pathspec-lstat-v1-1-8cf40b730146@gmail.com\n---\n builtin/ls-files.c | 11 +++++++++++\n t/meson.build | 1 +\n t/perf/p3010-ls-files.sh | 31 +++++++++++++++++++++++++++++++\n t/t3010-ls-files-killed-modified.sh | 18 ++++++++++++++++++\n 4 files changed, 61 insertions(+)\n\ndiff --git a/builtin/ls-files.c b/builtin/ls-files.c\nindex e1a22b41b9..8d7158652b 100644\n--- a/builtin/ls-files.c\n+++ b/builtin/ls-files.c\n@@ -450,6 +450,17 @@ static void show_files(struct repository *repo, struct dir_struct *dir)\n \t\t\tcontinue;\n \t\tif (ce_skip_worktree(ce))\n \t\t\tcontinue;\n+\t\t/*\n+\t\t * match_pathspec() is linear in pathspec.nr, so prefilter only\n+\t\t * the single-pathspec case. Only entries shown by show_ce()\n+\t\t * satisfy --error-unmatch.\n+\t\t */\n+\t\tif (pathspec.nr == 1 &&\n+\t\t !match_pathspec(repo->index, &pathspec, fullname.buf,\n+\t\t\t\t fullname.len, max_prefix_len, NULL,\n+\t\t\t\t S_ISDIR(ce->ce_mode) ||\n+\t\t\t\t S_ISGITLINK(ce->ce_mode)))\n+\t\t\tcontinue;\n \t\tstat_err = lstat(fullname.buf, &st);\n \t\tif (stat_err && (errno != ENOENT && errno != ENOTDIR))\n \t\t\terror_errno(\"cannot lstat '%s'\", fullname.buf);\ndiff --git a/t/meson.build b/t/meson.build\nindex 2af8d01279..ee8086e6ef 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -1140,6 +1140,7 @@ benchmarks = [\n 'perf/p1500-graph-walks.sh',\n 'perf/p1501-rev-parse-oneline.sh',\n 'perf/p2000-sparse-operations.sh',\n+ 'perf/p3010-ls-files.sh',\n 'perf/p3400-rebase.sh',\n 'perf/p3404-rebase-interactive.sh',\n 'perf/p4000-diff-algorithms.sh',\ndiff --git a/t/perf/p3010-ls-files.sh b/t/perf/p3010-ls-files.sh\nnew file mode 100755\nindex 0000000000..ae14449432\n--- /dev/null\n+++ b/t/perf/p3010-ls-files.sh\n@@ -0,0 +1,31 @@\n+#!/bin/sh\n+\n+test_description='Tests ls-files worktree performance'\n+\n+. ./perf-lib.sh\n+\n+test_perf_large_repo\n+test_checkout_worktree\n+\n+test_expect_success 'select a zero-prefix pathspec' '\n+\ttracked_file=$(git ls-files | sed -n 1p) &&\n+\ttest -n \"$tracked_file\" &&\n+\tpathspec=\"?${tracked_file#?}\" &&\n+\ttest_export pathspec\n+'\n+\n+test_perf 'ls-files --deleted with pathspec' '\n+\tgit -c core.fsmonitor=false ls-files --deleted \\\n+\t\t-- \"$pathspec\" >/dev/null\n+'\n+\n+test_perf 'ls-files --deleted with all-matching pathspec' '\n+\tgit -c core.fsmonitor=false ls-files --deleted -- \"*\" >/dev/null\n+'\n+\n+test_perf 'ls-files --modified with pathspec' '\n+\tgit -c core.fsmonitor=false ls-files --modified \\\n+\t\t-- \"$pathspec\" >/dev/null\n+'\n+\n+test_done\ndiff --git a/t/t3010-ls-files-killed-modified.sh b/t/t3010-ls-files-killed-modified.sh\nindex 7af4532cd1..6e38e10219 100755\n--- a/t/t3010-ls-files-killed-modified.sh\n+++ b/t/t3010-ls-files-killed-modified.sh\n@@ -124,4 +124,22 @@ test_expect_success 'validate git ls-files -m output.' '\n \ttest_cmp .expected .output\n '\n \n+test_expect_success 'worktree modes honor wildcard pathspecs' '\n+\tcat >.expected <<-\\EOF &&\n+\tpath2/file2\n+\tpath3/file3\n+\tEOF\n+\tgit ls-files --deleted -- \"path?/file?\" >.output &&\n+\ttest_cmp .expected .output &&\n+\n+\tcat >.expected <<-\\EOF &&\n+\tpath7\n+\tpath8\n+\tEOF\n+\tgit ls-files --modified --error-unmatch -- \"path[78]\" >.output &&\n+\ttest_cmp .expected .output &&\n+\n+\ttest_must_fail git ls-files --modified --error-unmatch -- path10\n+'\n+\n test_done\n\n---\nbase-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0\nchange-id: 20260607-ls-files-pathspec-lstat-885125a5d644\n\nBest regards,\n-- \nTamir Duberstein \n\n"},{"id":"545004","messageId":"xmqqv7bstmw8.fsf@gitster.g","threadId":"65777","inReplyTo":"20260608-ls-files-pathspec-lstat-v2-1-fb734b28422e@gmail.com","subject":"Re: [PATCH v2] ls-files: filter pathspec before lstat","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-06-09T03:26:15Z","receivedAt":"2026-06-09T03:26:18Z","isPatch":true,"body":"Tamir Duberstein writes:\n\n> show_files() checks whether each index entry is deleted or modified\n> before show_ce() applies the pathspec. prune_index() avoids most of this\n> work for pathspecs with a common directory prefix, but a top-level name\n> or leading wildcard leaves every entry to be checked.\n> ...\n\nPlease make sure that your v2 is a response to v1; otherwise loses\nsight of the previous iteration.\n\n> Changes in v2:\n> - Restrict early matching to one pathspec, avoiding the regression Jeff\n> demonstrated with many pathspecs.\n> - Add all-matching and many-pathspec performance results.\n> - Drop the Assisted-by trailer.\n> - Link to v1: https://patch.msgid.link/20260607-ls-files-pathspec-lstat-v1-1-8cf40b730146@gmail.com\n\nAnd it is *not* a replacement to force human to follow such a link.\n\nInstead, please make sure each piece of your e-mail identifies where\nit fits in the discussion thread by pointing the message of the\nprevious round with its In-Reply-To: header.\n\nThanks.\n"},{"id":"545005","messageId":"CAJ-ks9ku-uYeZ+3BhLAzrNdnOc7qnhudxQgy6PwU93jmr7ka+w@mail.gmail.com","threadId":"65777","inReplyTo":"xmqqv7bstmw8.fsf@gitster.g","subject":"Re: [PATCH v2] ls-files: filter pathspec before lstat","fromName":"Tamir Duberstein","fromEmail":"tamird@gmail.com","sentAt":"2026-06-09T03:38:47Z","receivedAt":"2026-06-09T03:39:26Z","isPatch":true,"body":"On Mon, Jun 8, 2026 at 8:26 PM Junio C Hamano wrote:\n>\n> Tamir Duberstein writes:\n>\n> > show_files() checks whether each index entry is deleted or modified\n> > before show_ce() applies the pathspec. prune_index() avoids most of this\n> > work for pathspecs with a common directory prefix, but a top-level name\n> > or leading wildcard leaves every entry to be checked.\n> > ...\n>\n> Please make sure that your v2 is a response to v1; otherwise loses\n> sight of the previous iteration.\n>\n> > Changes in v2:\n> > - Restrict early matching to one pathspec, avoiding the regression Jeff\n> > demonstrated with many pathspecs.\n> > - Add all-matching and many-pathspec performance results.\n> > - Drop the Assisted-by trailer.\n> > - Link to v1: https://patch.msgid.link/20260607-ls-files-pathspec-lstat-v1-1-8cf40b730146@gmail.com\n>\n> And it is *not* a replacement to force human to follow such a link.\n>\n> Instead, please make sure each piece of your e-mail identifies where\n> it fits in the discussion thread by pointing the message of the\n> previous round with its In-Reply-To: header.\n>\n> Thanks.\n\nApologies, I used b4 which follows kernel rules. I'll follow this\nguidance in the future.\n"},{"id":"545006","messageId":"xmqqecigtm5z.fsf@gitster.g","threadId":"65777","inReplyTo":"xmqqv7bstmw8.fsf@gitster.g","subject":"Re: [PATCH v2] ls-files: filter pathspec before lstat","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-06-09T03:42:00Z","receivedAt":"2026-06-09T03:42:02Z","isPatch":true,"body":"Junio C Hamano writes:\n\n> Please make sure that your v2 is a response to v1; otherwise loses\n> sight of the previous iteration.\n>\n>> Changes in v2:\n>> - Restrict early matching to one pathspec, avoiding the regression Jeff\n>> demonstrated with many pathspecs.\n>> - Add all-matching and many-pathspec performance results.\n>> - Drop the Assisted-by trailer.\n>> - Link to v1: https://patch.msgid.link/20260607-ls-files-pathspec-lstat-v1-1-8cf40b730146@gmail.com\n>\n> And it is *not* a replacement to force human to follow such a link.\n>\n> Instead, please make sure each piece of your e-mail identifies where\n> it fits in the discussion thread by pointing the message of the\n> previous round with its In-Reply-To: header.\n\nI won't complain about them individually, but it seems that all the\nother v2 in different topics from you share the same problem.\n\nDocumentation/SubmittingPatches expect that the messages on the same\ntopic are threaded with In-Reply-To: headers; e-mail based workflow\ntools like \"b4\" offer a useful feature that lets the user to feed\nthe message ID of an earlier round and fetch the messages in the\nlatest round. As the message IDs of an earlier round that have\nbecome commits for v1 are known in the refs/notes/amlog notes\n(published at the usual places), replacing a topic with its newer\niteration becomes:\n\n (0) check out the previous round.\n\n (1) learn the message ID of the previous round we have checked out\n using notes/amlog (e.g., \"git show -s --notes=amlog HEAD\"),\n\n (2) detach the HEAD at the base of the previous round (roughly \"git\n checkout master...HEAD\", but not always),\n\n (3) ask \"b4 am\" to fetch the latest round of the thread the message\n we found in step (1) belongs to, and apply these new patches,\n\n (4) run \"git range-diff @{-1}...HEAD\".\n\nwhich is very much automatable.\n\nUnless an author breaks the thread, that is.\n"},{"id":"545007","messageId":"CAJ-ks9nCK=a9s61yR7U9wf+e785Wir6RZSKTDWXyP9nH9aEXhQ@mail.gmail.com","threadId":"65777","inReplyTo":"xmqqecigtm5z.fsf@gitster.g","subject":"Re: [PATCH v2] ls-files: filter pathspec before lstat","fromName":"Tamir Duberstein","fromEmail":"tamird@gmail.com","sentAt":"2026-06-09T03:48:00Z","receivedAt":"2026-06-09T03:48:38Z","isPatch":true,"body":"On Mon, Jun 8, 2026 at 8:42 PM Junio C Hamano wrote:\n>\n> Junio C Hamano writes:\n>\n> > Please make sure that your v2 is a response to v1; otherwise loses\n> > sight of the previous iteration.\n> >\n> >> Changes in v2:\n> >> - Restrict early matching to one pathspec, avoiding the regression Jeff\n> >> demonstrated with many pathspecs.\n> >> - Add all-matching and many-pathspec performance results.\n> >> - Drop the Assisted-by trailer.\n> >> - Link to v1: https://patch.msgid.link/20260607-ls-files-pathspec-lstat-v1-1-8cf40b730146@gmail.com\n> >\n> > And it is *not* a replacement to force human to follow such a link.\n> >\n> > Instead, please make sure each piece of your e-mail identifies where\n> > it fits in the discussion thread by pointing the message of the\n> > previous round with its In-Reply-To: header.\n>\n> I won't complain about them individually, but it seems that all the\n> other v2 in different topics from you share the same problem.\n>\n> Documentation/SubmittingPatches expect that the messages on the same\n> topic are threaded with In-Reply-To: headers; e-mail based workflow\n> tools like \"b4\" offer a useful feature that lets the user to feed\n> the message ID of an earlier round and fetch the messages in the\n> latest round. As the message IDs of an earlier round that have\n> become commits for v1 are known in the refs/notes/amlog notes\n> (published at the usual places), replacing a topic with its newer\n> iteration becomes:\n>\n> (0) check out the previous round.\n>\n> (1) learn the message ID of the previous round we have checked out\n> using notes/amlog (e.g., \"git show -s --notes=amlog HEAD\"),\n>\n> (2) detach the HEAD at the base of the previous round (roughly \"git\n> checkout master...HEAD\", but not always),\n>\n> (3) ask \"b4 am\" to fetch the latest round of the thread the message\n> we found in step (1) belongs to, and apply these new patches,\n>\n> (4) run \"git range-diff @{-1}...HEAD\".\n>\n> which is very much automatable.\n>\n> Unless an author breaks the thread, that is.\n\nYes, heard loud and clear. As I mentioned on the other thread, I\nfollowed kernel conventions here by using b4. That's my fault. Sorry\nabout that. I'll do the proper thing in future mailings.\n"},{"id":"545056","messageId":"20260609104119.GA1509396@coredump.intra.peff.net","threadId":"65777","inReplyTo":"20260608-ls-files-pathspec-lstat-v2-1-fb734b28422e@gmail.com","subject":"Re: [PATCH v2] ls-files: filter pathspec before lstat","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-06-09T10:41:19Z","receivedAt":"2026-06-09T10:41:20Z","isPatch":true,"body":"On Mon, Jun 08, 2026 at 07:37:15PM -0700, Tamir Duberstein wrote:\n\n> +\t\t/*\n> +\t\t * match_pathspec() is linear in pathspec.nr, so prefilter only\n> +\t\t * the single-pathspec case. Only entries shown by show_ce()\n> +\t\t * satisfy --error-unmatch.\n> +\t\t */\n> +\t\tif (pathspec.nr == 1 &&\n> +\t\t !match_pathspec(repo->index, &pathspec, fullname.buf,\n> +\t\t\t\t fullname.len, max_prefix_len, NULL,\n> +\t\t\t\t S_ISDIR(ce->ce_mode) ||\n> +\t\t\t\t S_ISGITLINK(ce->ce_mode)))\n> +\t\t\tcontinue;\n\nThis feels...kind of arbitrary, no? Surely it's also faster with\npathspec.nr == 2, and so on up to some nr closer to the size of the\ntotal index. It feels weird to be making an arbitrary cutoff based on\npathspec performance in calling code like this.\n\nIt is not wrong, per se, as you are optimizing your case without trying\nto hurt any others. But what do we do when somebody profiles it and\ncomes along trying to bump the number to 2, or 10?\n\nI dunno.\n\n-Peff\n"},{"id":"545096","messageId":"CAJ-ks9mJk-=xp1hW77hAoZwwQAfpMukYO8OvvkLx646-2Z3_kg@mail.gmail.com","threadId":"65777","inReplyTo":"20260609104119.GA1509396@coredump.intra.peff.net","subject":"Re: [PATCH v2] ls-files: filter pathspec before lstat","fromName":"Tamir Duberstein","fromEmail":"tamird@gmail.com","sentAt":"2026-06-09T23:15:41Z","receivedAt":"2026-06-09T23:16:19Z","isPatch":true,"body":"On Tue, Jun 9, 2026 at 3:41 AM Jeff King wrote:\n>\n> On Mon, Jun 08, 2026 at 07:37:15PM -0700, Tamir Duberstein wrote:\n>\n> > + /*\n> > + * match_pathspec() is linear in pathspec.nr, so prefilter only\n> > + * the single-pathspec case. Only entries shown by show_ce()\n> > + * satisfy --error-unmatch.\n> > + */\n> > + if (pathspec.nr == 1 &&\n> > + !match_pathspec(repo->index, &pathspec, fullname.buf,\n> > + fullname.len, max_prefix_len, NULL,\n> > + S_ISDIR(ce->ce_mode) ||\n> > + S_ISGITLINK(ce->ce_mode)))\n> > + continue;\n>\n> This feels...kind of arbitrary, no? Surely it's also faster with\n> pathspec.nr == 2, and so on up to some nr closer to the size of the\n> total index. It feels weird to be making an arbitrary cutoff based on\n> pathspec performance in calling code like this.\n>\n> It is not wrong, per se, as you are optimizing your case without trying\n> to hurt any others. But what do we do when somebody profiles it and\n> comes along trying to bump the number to 2, or 10?\n>\n> I dunno.\n\nYeah, absolutely it's arbitrary. The simplest answer is that others\nare welcome to bump this, provided they make the case for it.\n"},{"id":"545257","messageId":"20260611084132.GK2191159@coredump.intra.peff.net","threadId":"65777","inReplyTo":"CAJ-ks9mJk-=xp1hW77hAoZwwQAfpMukYO8OvvkLx646-2Z3_kg@mail.gmail.com","subject":"Re: [PATCH v2] ls-files: filter pathspec before lstat","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-06-11T08:41:32Z","receivedAt":"2026-06-11T08:41:33Z","isPatch":true,"body":"On Tue, Jun 09, 2026 at 04:15:41PM -0700, Tamir Duberstein wrote:\n\n> On Tue, Jun 9, 2026 at 3:41 AM Jeff King wrote:\n> >\n> > On Mon, Jun 08, 2026 at 07:37:15PM -0700, Tamir Duberstein wrote:\n> >\n> > > + /*\n> > > + * match_pathspec() is linear in pathspec.nr, so prefilter only\n> > > + * the single-pathspec case. Only entries shown by show_ce()\n> > > + * satisfy --error-unmatch.\n> > > + */\n> > > + if (pathspec.nr == 1 &&\n> > > + !match_pathspec(repo->index, &pathspec, fullname.buf,\n> > > + fullname.len, max_prefix_len, NULL,\n> > > + S_ISDIR(ce->ce_mode) ||\n> > > + S_ISGITLINK(ce->ce_mode)))\n> > > + continue;\n> >\n> > This feels...kind of arbitrary, no? Surely it's also faster with\n> > pathspec.nr == 2, and so on up to some nr closer to the size of the\n> > total index. It feels weird to be making an arbitrary cutoff based on\n> > pathspec performance in calling code like this.\n> >\n> > It is not wrong, per se, as you are optimizing your case without trying\n> > to hurt any others. But what do we do when somebody profiles it and\n> > comes along trying to bump the number to 2, or 10?\n> >\n> > I dunno.\n> \n> Yeah, absolutely it's arbitrary. The simplest answer is that others\n> are welcome to bump this, provided they make the case for it.\n\nOK. I can live with, I suppose, but I am tempted to say that it should\njust kick in always (i.e., removing the pathspec.nr check).\n\nThough I did show a case where the performance regresses, it was pretty\nmade-up and not something I'd expect in the real world. And you'd see\nthat same crappy performance with \"git ls-files -- $(git ls-files)\",\nwithout the \"-m\". The real solution is making the pathspec code less\ncrappy.\n\n-Peff\n"},{"id":"545287","messageId":"CAJ-ks9kyPxGpBTQP4rBZaUpDYvyah8JpMx4mNPs8UbkddcHwOQ@mail.gmail.com","threadId":"65777","inReplyTo":"20260611084132.GK2191159@coredump.intra.peff.net","subject":"Re: [PATCH v2] ls-files: filter pathspec before lstat","fromName":"Tamir Duberstein","fromEmail":"tamird@gmail.com","sentAt":"2026-06-11T15:17:19Z","receivedAt":"2026-06-11T15:17:58Z","isPatch":true,"body":"On Thu, Jun 11, 2026 at 1:41 AM Jeff King wrote:\n>\n> On Tue, Jun 09, 2026 at 04:15:41PM -0700, Tamir Duberstein wrote:\n>\n> > On Tue, Jun 9, 2026 at 3:41 AM Jeff King wrote:\n> > >\n> > > On Mon, Jun 08, 2026 at 07:37:15PM -0700, Tamir Duberstein wrote:\n> > >\n> > > > + /*\n> > > > + * match_pathspec() is linear in pathspec.nr, so prefilter only\n> > > > + * the single-pathspec case. Only entries shown by show_ce()\n> > > > + * satisfy --error-unmatch.\n> > > > + */\n> > > > + if (pathspec.nr == 1 &&\n> > > > + !match_pathspec(repo->index, &pathspec, fullname.buf,\n> > > > + fullname.len, max_prefix_len, NULL,\n> > > > + S_ISDIR(ce->ce_mode) ||\n> > > > + S_ISGITLINK(ce->ce_mode)))\n> > > > + continue;\n> > >\n> > > This feels...kind of arbitrary, no? Surely it's also faster with\n> > > pathspec.nr == 2, and so on up to some nr closer to the size of the\n> > > total index. It feels weird to be making an arbitrary cutoff based on\n> > > pathspec performance in calling code like this.\n> > >\n> > > It is not wrong, per se, as you are optimizing your case without trying\n> > > to hurt any others. But what do we do when somebody profiles it and\n> > > comes along trying to bump the number to 2, or 10?\n> > >\n> > > I dunno.\n> >\n> > Yeah, absolutely it's arbitrary. The simplest answer is that others\n> > are welcome to bump this, provided they make the case for it.\n>\n> OK. I can live with, I suppose, but I am tempted to say that it should\n> just kick in always (i.e., removing the pathspec.nr check).\n>\n> Though I did show a case where the performance regresses, it was pretty\n> made-up and not something I'd expect in the real world. And you'd see\n> that same crappy performance with \"git ls-files -- $(git ls-files)\",\n> without the \"-m\". The real solution is making the pathspec code less\n> crappy.\n\nMaybe I can find time to look into this -- for now I'll treat this\nseries as not requiring further work.\n\nThanks!\nTamir\n"},{"id":"545306","messageId":"xmqqfr2tnfk0.fsf@gitster.g","threadId":"65777","inReplyTo":"20260611084132.GK2191159@coredump.intra.peff.net","subject":"Re: [PATCH v2] ls-files: filter pathspec before lstat","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-06-11T17:38:07Z","receivedAt":"2026-06-11T17:38:09Z","isPatch":true,"body":"Jeff King writes:\n\n>> Yeah, absolutely it's arbitrary. The simplest answer is that others\n>> are welcome to bump this, provided they make the case for it.\n>\n> OK. I can live with, I suppose, but I am tempted to say that it should\n> just kick in always (i.e., removing the pathspec.nr check).\n\nYeah, that is certainly simpler, and this ...\n\n> Though I did show a case where the performance regresses, it was pretty\n> made-up and not something I'd expect in the real world. And you'd see\n> that same crappy performance with \"git ls-files -- $(git ls-files)\",\n> without the \"-m\".\n\n... makes it clear that \"trigger only when there is one element in\nthe pathspec\" is optimizing for a wrong case.\n\nI think we want the log message document that this kind of thinking\nwent into the final choice of the heuristics, like, \"trigger only\nwhen there is one because ...\", or \"even though it would actually be\nan anti-optimization when the pathspec has enourmous number of\nelements, we always use this optimization because ...\", but as long\nas that is done, either solution is fine.\n\nThanks.\n"},{"id":"545326","messageId":"20260611-ls-files-pathspec-lstat-v3-1-f967e1a00c13@gmail.com","threadId":"65777","inReplyTo":"20260608-ls-files-pathspec-lstat-v2-1-fb734b28422e@gmail.com","subject":"[PATCH v3] ls-files: filter pathspec before lstat","fromName":"Tamir Duberstein","fromEmail":"tamird@gmail.com","sentAt":"2026-06-12T04:31:51Z","receivedAt":"2026-06-12T04:32:08Z","isPatch":true,"body":"In --deleted and --modified modes, show_files() calls lstat() for each\nindex entry before show_ce() applies the pathspec. prune_index() avoids\nmost of these calls for pathspecs with a common directory prefix, but\nnot for a top-level name or leading wildcard.\n\nMatch before lstat() to avoid accessing the worktree for entries that\ncannot be shown. Treat this as a prefilter: do not update ps_matched,\nand retain the match in show_ce() so --error-unmatch is satisfied only\nby entries that the selected modes actually show.\n\nPrefilter only a single pathspec item, bounding the added work for each\nindex entry. Applying match_pathspec() to multiple arguments can cost\nmore than the lstat() calls it avoids. In a synthetic repository with\n10,000 clean files, passing every path to ls-files --modified increased\nruntime from 112.5 ms to 494.1 ms when the prefilter was unconditional.\n\nWith $parent and $this exported as paths to binaries built from the\nparent and this commit, on a repository with 881,290 index entries:\n\n hyperfine --warmup 0 --runs 3 \\\n --command-name parent \\\n '$parent -c core.fsmonitor=false ls-files --deleted -- README.md >/dev/null' \\\n --command-name this-commit \\\n '$this -c core.fsmonitor=false ls-files --deleted -- README.md >/dev/null'\n\nreported means of 65.790 seconds for the parent and 4.987 seconds for\nthis commit.\n\nLink: https://lore.kernel.org/r/xmqqfr2tnfk0.fsf@gitster.g\nHelped-by: Jeff King \nSigned-off-by: Tamir Duberstein \n---\nA selective pathspec should let ls-files --deleted and --modified avoid\nstatting entries that cannot be shown. Match a single pathspec before\naccessing the worktree, while preserving the existing lstat-first order\nfor multiple pathspecs whose matching cost grows linearly.\n---\nChanges in v3:\n- Explain the conservative single-pathspec cutoff without referring to\n prior revisions.\n- Rerun the primary benchmark with the final implementation.\n- Make no code changes.\n- Link to v2: https://patch.msgid.link/20260608-ls-files-pathspec-lstat-v2-1-fb734b28422e@gmail.com\n\nChanges in v2:\n- Restrict early matching to one pathspec after measuring a regression\n with many pathspecs.\n- Add all-matching and many-pathspec performance results.\n- Drop the Assisted-by trailer.\n- Link to v1: https://patch.msgid.link/20260607-ls-files-pathspec-lstat-v1-1-8cf40b730146@gmail.com\n---\n builtin/ls-files.c | 11 +++++++++++\n t/meson.build | 1 +\n t/perf/p3010-ls-files.sh | 31 +++++++++++++++++++++++++++++++\n t/t3010-ls-files-killed-modified.sh | 18 ++++++++++++++++++\n 4 files changed, 61 insertions(+)\n\ndiff --git a/builtin/ls-files.c b/builtin/ls-files.c\nindex e1a22b41b9..8d7158652b 100644\n--- a/builtin/ls-files.c\n+++ b/builtin/ls-files.c\n@@ -450,6 +450,17 @@ static void show_files(struct repository *repo, struct dir_struct *dir)\n \t\t\tcontinue;\n \t\tif (ce_skip_worktree(ce))\n \t\t\tcontinue;\n+\t\t/*\n+\t\t * match_pathspec() is linear in pathspec.nr, so prefilter only\n+\t\t * the single-pathspec case. Only entries shown by show_ce()\n+\t\t * satisfy --error-unmatch.\n+\t\t */\n+\t\tif (pathspec.nr == 1 &&\n+\t\t !match_pathspec(repo->index, &pathspec, fullname.buf,\n+\t\t\t\t fullname.len, max_prefix_len, NULL,\n+\t\t\t\t S_ISDIR(ce->ce_mode) ||\n+\t\t\t\t S_ISGITLINK(ce->ce_mode)))\n+\t\t\tcontinue;\n \t\tstat_err = lstat(fullname.buf, &st);\n \t\tif (stat_err && (errno != ENOENT && errno != ENOTDIR))\n \t\t\terror_errno(\"cannot lstat '%s'\", fullname.buf);\ndiff --git a/t/meson.build b/t/meson.build\nindex 2af8d01279..ee8086e6ef 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -1140,6 +1140,7 @@ benchmarks = [\n 'perf/p1500-graph-walks.sh',\n 'perf/p1501-rev-parse-oneline.sh',\n 'perf/p2000-sparse-operations.sh',\n+ 'perf/p3010-ls-files.sh',\n 'perf/p3400-rebase.sh',\n 'perf/p3404-rebase-interactive.sh',\n 'perf/p4000-diff-algorithms.sh',\ndiff --git a/t/perf/p3010-ls-files.sh b/t/perf/p3010-ls-files.sh\nnew file mode 100755\nindex 0000000000..ae14449432\n--- /dev/null\n+++ b/t/perf/p3010-ls-files.sh\n@@ -0,0 +1,31 @@\n+#!/bin/sh\n+\n+test_description='Tests ls-files worktree performance'\n+\n+. ./perf-lib.sh\n+\n+test_perf_large_repo\n+test_checkout_worktree\n+\n+test_expect_success 'select a zero-prefix pathspec' '\n+\ttracked_file=$(git ls-files | sed -n 1p) &&\n+\ttest -n \"$tracked_file\" &&\n+\tpathspec=\"?${tracked_file#?}\" &&\n+\ttest_export pathspec\n+'\n+\n+test_perf 'ls-files --deleted with pathspec' '\n+\tgit -c core.fsmonitor=false ls-files --deleted \\\n+\t\t-- \"$pathspec\" >/dev/null\n+'\n+\n+test_perf 'ls-files --deleted with all-matching pathspec' '\n+\tgit -c core.fsmonitor=false ls-files --deleted -- \"*\" >/dev/null\n+'\n+\n+test_perf 'ls-files --modified with pathspec' '\n+\tgit -c core.fsmonitor=false ls-files --modified \\\n+\t\t-- \"$pathspec\" >/dev/null\n+'\n+\n+test_done\ndiff --git a/t/t3010-ls-files-killed-modified.sh b/t/t3010-ls-files-killed-modified.sh\nindex 7af4532cd1..6e38e10219 100755\n--- a/t/t3010-ls-files-killed-modified.sh\n+++ b/t/t3010-ls-files-killed-modified.sh\n@@ -124,4 +124,22 @@ test_expect_success 'validate git ls-files -m output.' '\n \ttest_cmp .expected .output\n '\n \n+test_expect_success 'worktree modes honor wildcard pathspecs' '\n+\tcat >.expected <<-\\EOF &&\n+\tpath2/file2\n+\tpath3/file3\n+\tEOF\n+\tgit ls-files --deleted -- \"path?/file?\" >.output &&\n+\ttest_cmp .expected .output &&\n+\n+\tcat >.expected <<-\\EOF &&\n+\tpath7\n+\tpath8\n+\tEOF\n+\tgit ls-files --modified --error-unmatch -- \"path[78]\" >.output &&\n+\ttest_cmp .expected .output &&\n+\n+\ttest_must_fail git ls-files --modified --error-unmatch -- path10\n+'\n+\n test_done\n\n---\nbase-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0\nchange-id: 20260607-ls-files-pathspec-lstat-885125a5d644\n\nBest regards,\n-- \nTamir Duberstein \n\n"},{"id":"545589","messageId":"xmqq4ij3ddst.fsf@gitster.g","threadId":"65777","inReplyTo":"20260611-ls-files-pathspec-lstat-v3-1-f967e1a00c13@gmail.com","subject":"Re: [PATCH v3] ls-files: filter pathspec before lstat","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-06-15T15:27:30Z","receivedAt":"2026-06-15T15:27:32Z","isPatch":true,"body":"Tamir Duberstein writes:\n\n> Prefilter only a single pathspec item, bounding the added work for each\n> index entry. Applying match_pathspec() to multiple arguments can cost\n> more than the lstat() calls it avoids. In a synthetic repository with\n> 10,000 clean files, passing every path to ls-files --modified increased\n> runtime from 112.5 ms to 494.1 ms when the prefilter was unconditional.\n\nI still think the choice of special casing a pathspec with a single\nelement is a lot harder to justify and invite people to start\ncomplaining \"why one and not three?\" than not special casing any\n(which makes the code simpler as well), as long as it is documented\nclearly, like the above paragraph, why the performance\ncharacteristics are so much different when pathspec has more than\none elments, the users and future developers can take it from there.\n\nSo let me mark the topic for 'next' now.\n\nThanks.\n\n"}]}