{"thread":{"id":"65659","subject":"[PATCH] stash: reuse cached index entries in --patch temporary index","startedAt":"2026-05-19T12:43:28Z","lastAt":"2026-06-01T21:33:15Z","messageCount":7,"participants":["Adam Johnson via GitGitGadget","Junio C Hamano","Adam Johnson"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"543638","messageId":"pull.2306.git.git.1779194605735.gitgitgadget@gmail.com","threadId":"65659","inReplyTo":null,"subject":"[PATCH] stash: reuse cached index entries in --patch temporary index","fromName":"Adam Johnson via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-05-19T12:43:25Z","receivedAt":"2026-05-19T12:43:28Z","isPatch":true,"body":"From: Adam Johnson <me@adamj.eu>\n\n`git stash -p` prepares the interactive selection by creating a\ntemporary index at HEAD, switching `GIT_INDEX_FILE` to it, and then\nrunning the `add -p` machinery.\n\nThat temporary index was created by running `git read-tree HEAD`.  The\nresulting index had no useful cached stat data or fsmonitor-valid bits\nfrom the real index.  When `run_add_p()` refreshed that temporary index\nbefore showing the first prompt, it could end up lstat(2)-ing every\ntracked file, even in a repository where `git diff` and `git restore -p`\ncan use fsmonitor to avoid that work.\n\nCreate the temporary index in-process instead.  Use `unpack_trees()` to\nreset the real index contents to HEAD while writing the result to the\ntemporary index path.  For paths whose index entries already match HEAD,\n`oneway_merge()` reuses the existing cache entries, preserving their\ncached stat data and `CE_FSMONITOR_VALID` state.\n\nThis makes the refresh performed by `run_add_p()` behave like the one\nused by `git restore -p`: unchanged paths can be skipped via fsmonitor\ninstead of being scanned again.\n\nIn a 206k file repository with `core.fsmonitor` enabled and a one-line\nchange in one file, time to first prompt dropped from 34.774 seconds to\n0.659 seconds.\n\nSigned-off-by: Adam Johnson <me@adamj.eu>\n---\n    stash: reuse cached index entries in --patch temporary index\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2306%2Fadamchainz%2Faj%2Foptimize-stash-patch-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2306/adamchainz/aj/optimize-stash-patch-v1\nPull-Request: https://github.com/git/git/pull/2306\n\n builtin/stash.c        | 71 ++++++++++++++++++++++++++++++++++++++----\n t/t3904-stash-patch.sh | 18 +++++++++++\n 2 files changed, 83 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex 32dbc97b47..48189cb9f7 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -372,6 +372,57 @@ static int reset_tree(struct object_id *i_tree, int update, int reset)\n \treturn 0;\n }\n \n+static int create_index_from_tree(const struct object_id *tree_id,\n+\t\t\t\t  const char *index_path)\n+{\n+\tint nr_trees = 1;\n+\tint ret = 0;\n+\tstruct unpack_trees_options opts;\n+\tstruct tree_desc t[MAX_UNPACK_TREES];\n+\tstruct tree *tree;\n+\tstruct index_state dst_istate = INDEX_STATE_INIT(the_repository);\n+\tstruct lock_file lock_file = LOCK_INIT;\n+\n+\trepo_read_index_preload(the_repository, NULL, 0);\n+\tif (refresh_index(the_repository->index, REFRESH_QUIET, NULL, NULL, NULL))\n+\t\treturn -1;\n+\n+\thold_lock_file_for_update(&lock_file, index_path, LOCK_DIE_ON_ERROR);\n+\n+\tmemset(&opts, 0, sizeof(opts));\n+\n+\ttree = repo_parse_tree_indirect(the_repository, tree_id);\n+\tif (!tree || repo_parse_tree(the_repository, tree)) {\n+\t\tret = -1;\n+\t\tgoto done;\n+\t}\n+\n+\tinit_tree_desc(t, &tree->object.oid, tree->buffer, tree->size);\n+\n+\topts.head_idx = 1;\n+\topts.src_index = the_repository->index;\n+\topts.dst_index = &dst_istate;\n+\topts.merge = 1;\n+\topts.reset = UNPACK_RESET_PROTECT_UNTRACKED;\n+\topts.fn = oneway_merge;\n+\n+\tif (unpack_trees(nr_trees, t, &opts)) {\n+\t\tret = -1;\n+\t\tgoto done;\n+\t}\n+\n+\tif (write_locked_index(&dst_istate, &lock_file, COMMIT_LOCK)) {\n+\t\tret = error(_(\"unable to write new index file\"));\n+\t\tgoto done;\n+\t}\n+\n+done:\n+\trelease_index(&dst_istate);\n+\tif (ret)\n+\t\trollback_lock_file(&lock_file);\n+\treturn ret;\n+}\n+\n static int diff_tree_binary(struct strbuf *out, struct object_id *w_commit)\n {\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n@@ -1321,18 +1372,26 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps,\n \t\t       struct interactive_options *interactive_opts)\n {\n \tint ret = 0;\n-\tstruct child_process cp_read_tree = CHILD_PROCESS_INIT;\n \tstruct child_process cp_diff_tree = CHILD_PROCESS_INIT;\n+\tstruct commit *head_commit;\n+\tconst struct object_id *head_tree;\n \tstruct index_state istate = INDEX_STATE_INIT(the_repository);\n \tchar *old_index_env = NULL, *old_repo_index_file;\n \n \tremove_path(stash_index_path.buf);\n \n-\tcp_read_tree.git_cmd = 1;\n-\tstrvec_pushl(&cp_read_tree.args, \"read-tree\", \"HEAD\", NULL);\n-\tstrvec_pushf(&cp_read_tree.env, \"GIT_INDEX_FILE=%s\",\n-\t\t     stash_index_path.buf);\n-\tif (run_command(&cp_read_tree)) {\n+\thead_commit = lookup_commit(the_repository, &info->b_commit);\n+\tif (!head_commit || repo_parse_commit(the_repository, head_commit)) {\n+\t\tret = -1;\n+\t\tgoto done;\n+\t}\n+\thead_tree = get_commit_tree_oid(head_commit);\n+\tif (!head_tree) {\n+\t\tret = -1;\n+\t\tgoto done;\n+\t}\n+\n+\tif (create_index_from_tree(head_tree, stash_index_path.buf)) {\n \t\tret = -1;\n \t\tgoto done;\n \t}\ndiff --git a/t/t3904-stash-patch.sh b/t/t3904-stash-patch.sh\nindex 90a4ff2c10..4b3241c8cd 100755\n--- a/t/t3904-stash-patch.sh\n+++ b/t/t3904-stash-patch.sh\n@@ -84,6 +84,24 @@ test_expect_success 'none of this moved HEAD' '\n \tverify_saved_head\n '\n \n+test_expect_success 'stash -p with unmodified tracked files present' '\n+\tgit reset --hard &&\n+\techo line1 >alpha &&\n+\techo line1 >beta &&\n+\tgit add alpha beta &&\n+\tgit commit -m \"add alpha and beta\" &&\n+\techo line2 >>alpha &&\n+\techo y | git stash -p &&\n+\techo line1 >expect &&\n+\ttest_cmp expect alpha &&\n+\ttest_cmp expect beta &&\n+\tgit stash pop &&\n+\tprintf \"line1\\nline2\\n\" >expect &&\n+\ttest_cmp expect alpha &&\n+\techo line1 >expect &&\n+\ttest_cmp expect beta\n+'\n+\n test_expect_success 'stash -p with split hunk' '\n \tgit reset --hard &&\n \tcat >test <<-\\EOF &&\n\nbase-commit: 7bcaabddcf68bd0702697da5904c3b68c52f94cf\n-- \ngitgitgadget\n"},{"id":"543719","messageId":"xmqqse7m6deh.fsf@gitster.g","threadId":"65659","inReplyTo":"pull.2306.git.git.1779194605735.gitgitgadget@gmail.com","subject":"Re: [PATCH] stash: reuse cached index entries in --patch temporary index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-05-20T02:08:38Z","receivedAt":"2026-05-20T02:08:42Z","isPatch":true,"body":"\"Adam Johnson via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Adam Johnson <me@adamj.eu>\n>\n> `git stash -p` prepares the interactive selection by creating a\n> temporary index at HEAD, switching `GIT_INDEX_FILE` to it, and then\n> running the `add -p` machinery.\n>\n> That temporary index was created by running `git read-tree HEAD`.  The\n> resulting index had no useful cached stat data or fsmonitor-valid bits\n> from the real index.  When `run_add_p()` refreshed that temporary index\n> before showing the first prompt, it could end up lstat(2)-ing every\n> tracked file, even in a repository where `git diff` and `git restore -p`\n> can use fsmonitor to avoid that work.\n>\n> Create the temporary index in-process instead.  Use `unpack_trees()` to\n> reset the real index contents to HEAD while writing the result to the\n> temporary index path.  For paths whose index entries already match HEAD,\n> `oneway_merge()` reuses the existing cache entries, preserving their\n> cached stat data and `CE_FSMONITOR_VALID` state.\n\nClever.  As the fsmonitor_valid bit is in-core only, updating the\nindex in-process would be an obvious and probably the only sensible\nway to preserve it.\n\nI however have to wonder if simply replacing the external process\ninvocation with \"git read-tree -m HEAD\" (i.e., oneway merge) gives\na similar speed-up.\n\n> This makes the refresh performed by `run_add_p()` behave like the one\n> used by `git restore -p`: unchanged paths can be skipped via fsmonitor\n> instead of being scanned again.\n>\n> In a 206k file repository with `core.fsmonitor` enabled and a one-line\n> change in one file, time to first prompt dropped from 34.774 seconds to\n> 0.659 seconds.\n\nInteresting.\n\n> diff --git a/t/t3904-stash-patch.sh b/t/t3904-stash-patch.sh\n> index 90a4ff2c10..4b3241c8cd 100755\n> --- a/t/t3904-stash-patch.sh\n> +++ b/t/t3904-stash-patch.sh\n> @@ -84,6 +84,24 @@ test_expect_success 'none of this moved HEAD' '\n>  \tverify_saved_head\n>  '\n>  \n> +test_expect_success 'stash -p with unmodified tracked files present' '\n> +\tgit reset --hard &&\n> +\techo line1 >alpha &&\n> +\techo line1 >beta &&\n> +\tgit add alpha beta &&\n> +\tgit commit -m \"add alpha and beta\" &&\n> +\techo line2 >>alpha &&\n> +\techo y | git stash -p &&\n> +\techo line1 >expect &&\n> +\ttest_cmp expect alpha &&\n> +\ttest_cmp expect beta &&\n> +\tgit stash pop &&\n> +\tprintf \"line1\\nline2\\n\" >expect &&\n> +\ttest_cmp expect alpha &&\n> +\techo line1 >expect &&\n> +\ttest_cmp expect beta\n> +'\n\nWhat I read from the proposed log message is that the change is\npurely about performance and should not change any behaviour.  Why\ndo we need a new test in t/t3904?  I would not have surprised if we\nsaw a new test in t/perf/, though.\n\nThanks.\n"},{"id":"543720","messageId":"xmqqldde6cl5.fsf@gitster.g","threadId":"65659","inReplyTo":"pull.2306.git.git.1779194605735.gitgitgadget@gmail.com","subject":"Re: [PATCH] stash: reuse cached index entries in --patch temporary index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-05-20T02:26:14Z","receivedAt":"2026-05-20T02:26:19Z","isPatch":true,"body":"\"Adam Johnson via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n>  2 files changed, 83 insertions(+), 6 deletions(-)\n>\n> diff --git a/builtin/stash.c b/builtin/stash.c\n> index 32dbc97b47..48189cb9f7 100644\n> --- a/builtin/stash.c\n> +++ b/builtin/stash.c\n> @@ -372,6 +372,57 @@ static int reset_tree(struct object_id *i_tree, int update, int reset)\n>  \treturn 0;\n>  }\n>  \n> +static int create_index_from_tree(const struct object_id *tree_id,\n> +\t\t\t\t  const char *index_path)\n> +{\n> +\tint nr_trees = 1;\n> +\tint ret = 0;\n> +\tstruct unpack_trees_options opts;\n> +\tstruct tree_desc t[MAX_UNPACK_TREES];\n> +\tstruct tree *tree;\n> +\tstruct index_state dst_istate = INDEX_STATE_INIT(the_repository);\n> +\tstruct lock_file lock_file = LOCK_INIT;\n> +\n> +\trepo_read_index_preload(the_repository, NULL, 0);\n> +\tif (refresh_index(the_repository->index, REFRESH_QUIET, NULL, NULL, NULL))\n> +\t\treturn -1;\n\nIs this \"non-zero return from refresh_index() leads to a failure\"\nintended?  The old \"git read-tree HEAD\" wouldn't have cared if the\noriginal index were unmerged, for example, but with this update, we\nwill see an immediate failure.  There are other conditions that\nrefresh_index() flips its local variable has_errors on, which leads\nto its non-zero return.\n\nSince \"git stash -p\" is almost always invoked when the user has\nunstaged modifications, I am not sure allowing refresh_index() to\nnotice and barf is what we want here.\n"},{"id":"543953","messageId":"9e2058b9-4c0d-4c4b-8a65-0eb4869a815c@app.fastmail.com","threadId":"65659","inReplyTo":"xmqqse7m6deh.fsf@gitster.g","subject":"Re: [PATCH] stash: reuse cached index entries in --patch temporary index","fromName":"Adam Johnson","fromEmail":"me@adamj.eu","sentAt":"2026-05-22T20:53:53Z","receivedAt":"2026-05-22T20:54:14Z","isPatch":true,"body":"> I however have to wonder if simply replacing the external process\n> invocation with \"git read-tree -m HEAD\" (i.e., oneway merge) gives\n> a similar speed-up.\n\nGood idea, I just tried this, but it does not help. The subprocess runs\nwith GIT_INDEX_FILE set to a temporary index, so oneway_merge\nnever uses the CE_FSMONITOR_VALID fast path.\n\nThe in-process approach is necessary because it lets us set\nopts.src_index to the real index with cached stat and fsmonitor data,\nbefore switching GIT_INDEX_FILE.\n\n> What I read from the proposed log message is that the change is\n> purely about performance and should not change any behaviour.  Why\n> do we need a new test in t/t3904?  I would not have surprised if we\n> saw a new test in t/perf/, though.\n\nAh yeah, my bad. I added this while iterating to catch a bug I introduced,\nbut it's not necessary for the final patch. Will remove.\n\nOn Wed, 20 May 2026, at 03:08, Junio C Hamano wrote:\n> \"Adam Johnson via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n> \n> > From: Adam Johnson <me@adamj.eu>\n> >\n> > `git stash -p` prepares the interactive selection by creating a\n> > temporary index at HEAD, switching `GIT_INDEX_FILE` to it, and then\n> > running the `add -p` machinery.\n> >\n> > That temporary index was created by running `git read-tree HEAD`.  The\n> > resulting index had no useful cached stat data or fsmonitor-valid bits\n> > from the real index.  When `run_add_p()` refreshed that temporary index\n> > before showing the first prompt, it could end up lstat(2)-ing every\n> > tracked file, even in a repository where `git diff` and `git restore -p`\n> > can use fsmonitor to avoid that work.\n> >\n> > Create the temporary index in-process instead.  Use `unpack_trees()` to\n> > reset the real index contents to HEAD while writing the result to the\n> > temporary index path.  For paths whose index entries already match HEAD,\n> > `oneway_merge()` reuses the existing cache entries, preserving their\n> > cached stat data and `CE_FSMONITOR_VALID` state.\n> \n> Clever.  As the fsmonitor_valid bit is in-core only, updating the\n> index in-process would be an obvious and probably the only sensible\n> way to preserve it.\n> \n> I however have to wonder if simply replacing the external process\n> invocation with \"git read-tree -m HEAD\" (i.e., oneway merge) gives\n> a similar speed-up.\n> \n> > This makes the refresh performed by `run_add_p()` behave like the one\n> > used by `git restore -p`: unchanged paths can be skipped via fsmonitor\n> > instead of being scanned again.\n> >\n> > In a 206k file repository with `core.fsmonitor` enabled and a one-line\n> > change in one file, time to first prompt dropped from 34.774 seconds to\n> > 0.659 seconds.\n> \n> Interesting.\n> \n> > diff --git a/t/t3904-stash-patch.sh b/t/t3904-stash-patch.sh\n> > index 90a4ff2c10..4b3241c8cd 100755\n> > --- a/t/t3904-stash-patch.sh\n> > +++ b/t/t3904-stash-patch.sh\n> > @@ -84,6 +84,24 @@ test_expect_success 'none of this moved HEAD' '\n> >  verify_saved_head\n> >  '\n> >  \n> > +test_expect_success 'stash -p with unmodified tracked files present' '\n> > + git reset --hard &&\n> > + echo line1 >alpha &&\n> > + echo line1 >beta &&\n> > + git add alpha beta &&\n> > + git commit -m \"add alpha and beta\" &&\n> > + echo line2 >>alpha &&\n> > + echo y | git stash -p &&\n> > + echo line1 >expect &&\n> > + test_cmp expect alpha &&\n> > + test_cmp expect beta &&\n> > + git stash pop &&\n> > + printf \"line1\\nline2\\n\" >expect &&\n> > + test_cmp expect alpha &&\n> > + echo line1 >expect &&\n> > + test_cmp expect beta\n> > +'\n> \n> What I read from the proposed log message is that the change is\n> purely about performance and should not change any behaviour.  Why\n> do we need a new test in t/t3904?  I would not have surprised if we\n> saw a new test in t/perf/, though.\n> \n> Thanks.\n> \n"},{"id":"543954","messageId":"e6e3ba3a-a08d-426b-b0ae-1f57554b2b1d@app.fastmail.com","threadId":"65659","inReplyTo":"xmqqldde6cl5.fsf@gitster.g","subject":"Re: [PATCH] stash: reuse cached index entries in --patch temporary index","fromName":"Adam Johnson","fromEmail":"me@adamj.eu","sentAt":"2026-05-22T20:55:27Z","receivedAt":"2026-05-22T20:55:49Z","isPatch":true,"body":"> Is this \"non-zero return from refresh_index() leads to a failure\"\n> intended?\n\nGood catch, it’s not needed. Removing, we can make the call\nunconditional.\n\nOn Wed, 20 May 2026, at 03:26, Junio C Hamano wrote:\n> \"Adam Johnson via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n> \n> >  2 files changed, 83 insertions(+), 6 deletions(-)\n> >\n> > diff --git a/builtin/stash.c b/builtin/stash.c\n> > index 32dbc97b47..48189cb9f7 100644\n> > --- a/builtin/stash.c\n> > +++ b/builtin/stash.c\n> > @@ -372,6 +372,57 @@ static int reset_tree(struct object_id *i_tree, int update, int reset)\n> >  return 0;\n> >  }\n> >  \n> > +static int create_index_from_tree(const struct object_id *tree_id,\n> > +   const char *index_path)\n> > +{\n> > + int nr_trees = 1;\n> > + int ret = 0;\n> > + struct unpack_trees_options opts;\n> > + struct tree_desc t[MAX_UNPACK_TREES];\n> > + struct tree *tree;\n> > + struct index_state dst_istate = INDEX_STATE_INIT(the_repository);\n> > + struct lock_file lock_file = LOCK_INIT;\n> > +\n> > + repo_read_index_preload(the_repository, NULL, 0);\n> > + if (refresh_index(the_repository->index, REFRESH_QUIET, NULL, NULL, NULL))\n> > + return -1;\n> \n> Is this \"non-zero return from refresh_index() leads to a failure\"\n> intended?  The old \"git read-tree HEAD\" wouldn't have cared if the\n> original index were unmerged, for example, but with this update, we\n> will see an immediate failure.  There are other conditions that\n> refresh_index() flips its local variable has_errors on, which leads\n> to its non-zero return.\n> \n> Since \"git stash -p\" is almost always invoked when the user has\n> unstaged modifications, I am not sure allowing refresh_index() to\n> notice and barf is what we want here.\n> \n"},{"id":"543957","messageId":"pull.2306.v2.git.git.1779491545531.gitgitgadget@gmail.com","threadId":"65659","inReplyTo":"pull.2306.git.git.1779194605735.gitgitgadget@gmail.com","subject":"[PATCH v2] stash: reuse cached index entries in --patch temporary index","fromName":"Adam Johnson via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-05-22T23:12:25Z","receivedAt":"2026-05-22T23:12:28Z","isPatch":true,"body":"From: Adam Johnson <me@adamj.eu>\n\n`git stash -p` prepares the interactive selection by creating a\ntemporary index at HEAD, switching `GIT_INDEX_FILE` to it, and then\nrunning the `add -p` machinery.\n\nThat temporary index was created by running `git read-tree HEAD`.  The\nresulting index had no useful cached stat data or fsmonitor-valid bits\nfrom the real index.  When `run_add_p()` refreshed that temporary index\nbefore showing the first prompt, it could end up lstat(2)-ing every\ntracked file, even in a repository where `git diff` and `git restore -p`\ncan use fsmonitor to avoid that work.\n\nCreate the temporary index in-process instead.  Use `unpack_trees()` to\nreset the real index contents to HEAD while writing the result to the\ntemporary index path.  For paths whose index entries already match HEAD,\n`oneway_merge()` reuses the existing cache entries, preserving their\ncached stat data and `CE_FSMONITOR_VALID` state.\n\nThis makes the refresh performed by `run_add_p()` behave like the one\nused by `git restore -p`: unchanged paths can be skipped via fsmonitor\ninstead of being scanned again.\n\nIn a 206k file repository with `core.fsmonitor` enabled and a one-line\nchange in one file, time to first prompt dropped from 34.774 seconds to\n0.659 seconds. The new perf test file demonstrates similar improvements,\nwith maen times for without- and with-fsmonitor cases dropping from 6.90\nand 6.83 seconds to 0.55 and 0.28 seconds, respectively.\n\nSigned-off-by: Adam Johnson <me@adamj.eu>\n---\n    stash: reuse cached index entries in --patch temporary index\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2306%2Fadamchainz%2Faj%2Foptimize-stash-patch-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2306/adamchainz/aj/optimize-stash-patch-v2\nPull-Request: https://github.com/git/git/pull/2306\n\nRange-diff vs v1:\n\n 1:  b228160cc4 ! 1:  8785572c4d stash: reuse cached index entries in --patch temporary index\n     @@ Commit message\n      \n          In a 206k file repository with `core.fsmonitor` enabled and a one-line\n          change in one file, time to first prompt dropped from 34.774 seconds to\n     -    0.659 seconds.\n     +    0.659 seconds. The new perf test file demonstrates similar improvements,\n     +    with maen times for without- and with-fsmonitor cases dropping from 6.90\n     +    and 6.83 seconds to 0.55 and 0.28 seconds, respectively.\n      \n          Signed-off-by: Adam Johnson <me@adamj.eu>\n      \n     @@ builtin/stash.c: static int reset_tree(struct object_id *i_tree, int update, int\n      +\tstruct lock_file lock_file = LOCK_INIT;\n      +\n      +\trepo_read_index_preload(the_repository, NULL, 0);\n     -+\tif (refresh_index(the_repository->index, REFRESH_QUIET, NULL, NULL, NULL))\n     -+\t\treturn -1;\n     ++\trefresh_index(the_repository->index, REFRESH_QUIET, NULL, NULL, NULL);\n      +\n      +\thold_lock_file_for_update(&lock_file, index_path, LOCK_DIE_ON_ERROR);\n      +\n     @@ builtin/stash.c: static int stash_patch(struct stash_info *info, const struct pa\n       \t\tgoto done;\n       \t}\n      \n     - ## t/t3904-stash-patch.sh ##\n     -@@ t/t3904-stash-patch.sh: test_expect_success 'none of this moved HEAD' '\n     - \tverify_saved_head\n     - '\n     - \n     -+test_expect_success 'stash -p with unmodified tracked files present' '\n     -+\tgit reset --hard &&\n     -+\techo line1 >alpha &&\n     -+\techo line1 >beta &&\n     -+\tgit add alpha beta &&\n     -+\tgit commit -m \"add alpha and beta\" &&\n     -+\techo line2 >>alpha &&\n     -+\techo y | git stash -p &&\n     -+\techo line1 >expect &&\n     -+\ttest_cmp expect alpha &&\n     -+\ttest_cmp expect beta &&\n     -+\tgit stash pop &&\n     -+\tprintf \"line1\\nline2\\n\" >expect &&\n     -+\ttest_cmp expect alpha &&\n     -+\techo line1 >expect &&\n     -+\ttest_cmp expect beta\n     + ## t/perf/p3904-stash-patch.sh (new) ##\n     +@@\n     ++#!/bin/sh\n     ++\n     ++test_description=\"Performance tests for git stash -p\"\n     ++\n     ++. ./perf-lib.sh\n     ++\n     ++test_perf_fresh_repo\n     ++\n     ++test_expect_success \"setup\" '\n     ++\tmkdir files &&\n     ++\ttest_seq 1 100000 | while read i; do\n     ++\t\techo \"content $i\" >files/$i.txt || return 1\n     ++\tdone &&\n     ++\tgit add files/ &&\n     ++\tgit commit -q -m \"add tracked files\" &&\n     ++\techo modified >files/1.txt\n      +'\n      +\n     - test_expect_success 'stash -p with split hunk' '\n     - \tgit reset --hard &&\n     - \tcat >test <<-\\EOF &&\n     ++test_perf \"stash -p, no fsmonitor\" \\\n     ++\t--setup 'echo modified >files/1.txt' '\n     ++\tprintf \"q\\n\" | git stash -p >/dev/null 2>&1 || true\n     ++'\n     ++\n     ++if test_have_prereq FSMONITOR_DAEMON\n     ++then\n     ++\ttest_expect_success \"enable builtin fsmonitor\" '\n     ++\t\tgit config core.fsmonitor true &&\n     ++\t\tgit fsmonitor--daemon start &&\n     ++\t\tgit update-index --fsmonitor &&\n     ++\t\tgit status >/dev/null 2>&1\n     ++\t'\n     ++\n     ++\ttest_perf \"stash -p, builtin fsmonitor\" \\\n     ++\t\t--setup 'echo modified >files/1.txt && git status >/dev/null 2>&1' '\n     ++\t\tprintf \"q\\n\" | git stash -p >/dev/null 2>&1 || true\n     ++\t'\n     ++\n     ++\ttest_expect_success \"stop builtin fsmonitor\" '\n     ++\t\tgit fsmonitor--daemon stop\n     ++\t'\n     ++fi\n     ++\n     ++test_done\n\n\n builtin/stash.c             | 70 +++++++++++++++++++++++++++++++++----\n t/perf/p3904-stash-patch.sh | 43 +++++++++++++++++++++++\n 2 files changed, 107 insertions(+), 6 deletions(-)\n create mode 100755 t/perf/p3904-stash-patch.sh\n\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex 32dbc97b47..c4809f299a 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -372,6 +372,56 @@ static int reset_tree(struct object_id *i_tree, int update, int reset)\n \treturn 0;\n }\n \n+static int create_index_from_tree(const struct object_id *tree_id,\n+\t\t\t\t  const char *index_path)\n+{\n+\tint nr_trees = 1;\n+\tint ret = 0;\n+\tstruct unpack_trees_options opts;\n+\tstruct tree_desc t[MAX_UNPACK_TREES];\n+\tstruct tree *tree;\n+\tstruct index_state dst_istate = INDEX_STATE_INIT(the_repository);\n+\tstruct lock_file lock_file = LOCK_INIT;\n+\n+\trepo_read_index_preload(the_repository, NULL, 0);\n+\trefresh_index(the_repository->index, REFRESH_QUIET, NULL, NULL, NULL);\n+\n+\thold_lock_file_for_update(&lock_file, index_path, LOCK_DIE_ON_ERROR);\n+\n+\tmemset(&opts, 0, sizeof(opts));\n+\n+\ttree = repo_parse_tree_indirect(the_repository, tree_id);\n+\tif (!tree || repo_parse_tree(the_repository, tree)) {\n+\t\tret = -1;\n+\t\tgoto done;\n+\t}\n+\n+\tinit_tree_desc(t, &tree->object.oid, tree->buffer, tree->size);\n+\n+\topts.head_idx = 1;\n+\topts.src_index = the_repository->index;\n+\topts.dst_index = &dst_istate;\n+\topts.merge = 1;\n+\topts.reset = UNPACK_RESET_PROTECT_UNTRACKED;\n+\topts.fn = oneway_merge;\n+\n+\tif (unpack_trees(nr_trees, t, &opts)) {\n+\t\tret = -1;\n+\t\tgoto done;\n+\t}\n+\n+\tif (write_locked_index(&dst_istate, &lock_file, COMMIT_LOCK)) {\n+\t\tret = error(_(\"unable to write new index file\"));\n+\t\tgoto done;\n+\t}\n+\n+done:\n+\trelease_index(&dst_istate);\n+\tif (ret)\n+\t\trollback_lock_file(&lock_file);\n+\treturn ret;\n+}\n+\n static int diff_tree_binary(struct strbuf *out, struct object_id *w_commit)\n {\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n@@ -1321,18 +1371,26 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps,\n \t\t       struct interactive_options *interactive_opts)\n {\n \tint ret = 0;\n-\tstruct child_process cp_read_tree = CHILD_PROCESS_INIT;\n \tstruct child_process cp_diff_tree = CHILD_PROCESS_INIT;\n+\tstruct commit *head_commit;\n+\tconst struct object_id *head_tree;\n \tstruct index_state istate = INDEX_STATE_INIT(the_repository);\n \tchar *old_index_env = NULL, *old_repo_index_file;\n \n \tremove_path(stash_index_path.buf);\n \n-\tcp_read_tree.git_cmd = 1;\n-\tstrvec_pushl(&cp_read_tree.args, \"read-tree\", \"HEAD\", NULL);\n-\tstrvec_pushf(&cp_read_tree.env, \"GIT_INDEX_FILE=%s\",\n-\t\t     stash_index_path.buf);\n-\tif (run_command(&cp_read_tree)) {\n+\thead_commit = lookup_commit(the_repository, &info->b_commit);\n+\tif (!head_commit || repo_parse_commit(the_repository, head_commit)) {\n+\t\tret = -1;\n+\t\tgoto done;\n+\t}\n+\thead_tree = get_commit_tree_oid(head_commit);\n+\tif (!head_tree) {\n+\t\tret = -1;\n+\t\tgoto done;\n+\t}\n+\n+\tif (create_index_from_tree(head_tree, stash_index_path.buf)) {\n \t\tret = -1;\n \t\tgoto done;\n \t}\ndiff --git a/t/perf/p3904-stash-patch.sh b/t/perf/p3904-stash-patch.sh\nnew file mode 100755\nindex 0000000000..4cfce638be\n--- /dev/null\n+++ b/t/perf/p3904-stash-patch.sh\n@@ -0,0 +1,43 @@\n+#!/bin/sh\n+\n+test_description=\"Performance tests for git stash -p\"\n+\n+. ./perf-lib.sh\n+\n+test_perf_fresh_repo\n+\n+test_expect_success \"setup\" '\n+\tmkdir files &&\n+\ttest_seq 1 100000 | while read i; do\n+\t\techo \"content $i\" >files/$i.txt || return 1\n+\tdone &&\n+\tgit add files/ &&\n+\tgit commit -q -m \"add tracked files\" &&\n+\techo modified >files/1.txt\n+'\n+\n+test_perf \"stash -p, no fsmonitor\" \\\n+\t--setup 'echo modified >files/1.txt' '\n+\tprintf \"q\\n\" | git stash -p >/dev/null 2>&1 || true\n+'\n+\n+if test_have_prereq FSMONITOR_DAEMON\n+then\n+\ttest_expect_success \"enable builtin fsmonitor\" '\n+\t\tgit config core.fsmonitor true &&\n+\t\tgit fsmonitor--daemon start &&\n+\t\tgit update-index --fsmonitor &&\n+\t\tgit status >/dev/null 2>&1\n+\t'\n+\n+\ttest_perf \"stash -p, builtin fsmonitor\" \\\n+\t\t--setup 'echo modified >files/1.txt && git status >/dev/null 2>&1' '\n+\t\tprintf \"q\\n\" | git stash -p >/dev/null 2>&1 || true\n+\t'\n+\n+\ttest_expect_success \"stop builtin fsmonitor\" '\n+\t\tgit fsmonitor--daemon stop\n+\t'\n+fi\n+\n+test_done\n\nbase-commit: 7bcaabddcf68bd0702697da5904c3b68c52f94cf\n-- \ngitgitgadget\n"},{"id":"544424","messageId":"xmqqcxya3q07.fsf@gitster.g","threadId":"65659","inReplyTo":"pull.2306.v2.git.git.1779491545531.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] stash: reuse cached index entries in --patch temporary index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-06-01T21:33:12Z","receivedAt":"2026-06-01T21:33:15Z","isPatch":true,"body":"\"Adam Johnson via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Adam Johnson <me@adamj.eu>\n>\n> `git stash -p` prepares the interactive selection by creating a\n> temporary index at HEAD, switching `GIT_INDEX_FILE` to it, and then\n> running the `add -p` machinery.\n>\n> That temporary index was created by running `git read-tree HEAD`.  The\n> resulting index had no useful cached stat data or fsmonitor-valid bits\n> from the real index.  When `run_add_p()` refreshed that temporary index\n> before showing the first prompt, it could end up lstat(2)-ing every\n> tracked file, even in a repository where `git diff` and `git restore -p`\n> can use fsmonitor to avoid that work.\n>\n> Create the temporary index in-process instead.  Use `unpack_trees()` to\n> reset the real index contents to HEAD while writing the result to the\n> temporary index path.  For paths whose index entries already match HEAD,\n> `oneway_merge()` reuses the existing cache entries, preserving their\n> cached stat data and `CE_FSMONITOR_VALID` state.\n>\n> This makes the refresh performed by `run_add_p()` behave like the one\n> used by `git restore -p`: unchanged paths can be skipped via fsmonitor\n> instead of being scanned again.\n>\n> In a 206k file repository with `core.fsmonitor` enabled and a one-line\n> change in one file, time to first prompt dropped from 34.774 seconds to\n> 0.659 seconds. The new perf test file demonstrates similar improvements,\n> with maen times for without- and with-fsmonitor cases dropping from 6.90\n> and 6.83 seconds to 0.55 and 0.28 seconds, respectively.\n>\n> Signed-off-by: Adam Johnson <me@adamj.eu>\n> ---\n>     stash: reuse cached index entries in --patch temporary index\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2306%2Fadamchainz%2Faj%2Foptimize-stash-patch-v2\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2306/adamchainz/aj/optimize-stash-patch-v2\n> Pull-Request: https://github.com/git/git/pull/2306\n\nThe diff relative to the previous round looked good.  I am not a\n\"stash -p\" user myself, but I suspect that there are people who\nheavily use it, so I'd feel safer if an extra set of eye looks at\nthe patch and gives an Ack, but other than that I have no comments\non the patch.  Looking good.\n\nThanks.\n\n\n>  builtin/stash.c             | 70 +++++++++++++++++++++++++++++++++----\n>  t/perf/p3904-stash-patch.sh | 43 +++++++++++++++++++++++\n>  2 files changed, 107 insertions(+), 6 deletions(-)\n>  create mode 100755 t/perf/p3904-stash-patch.sh\n>\n> diff --git a/builtin/stash.c b/builtin/stash.c\n> index 32dbc97b47..c4809f299a 100644\n> --- a/builtin/stash.c\n> +++ b/builtin/stash.c\n> @@ -372,6 +372,56 @@ static int reset_tree(struct object_id *i_tree, int update, int reset)\n>  \treturn 0;\n>  }\n>  \n> +static int create_index_from_tree(const struct object_id *tree_id,\n> +\t\t\t\t  const char *index_path)\n> +{\n> +\tint nr_trees = 1;\n> +\tint ret = 0;\n> +\tstruct unpack_trees_options opts;\n> +\tstruct tree_desc t[MAX_UNPACK_TREES];\n> +\tstruct tree *tree;\n> +\tstruct index_state dst_istate = INDEX_STATE_INIT(the_repository);\n> +\tstruct lock_file lock_file = LOCK_INIT;\n> +\n> +\trepo_read_index_preload(the_repository, NULL, 0);\n> +\trefresh_index(the_repository->index, REFRESH_QUIET, NULL, NULL, NULL);\n> +\n> +\thold_lock_file_for_update(&lock_file, index_path, LOCK_DIE_ON_ERROR);\n> +\n> +\tmemset(&opts, 0, sizeof(opts));\n> +\n> +\ttree = repo_parse_tree_indirect(the_repository, tree_id);\n> +\tif (!tree || repo_parse_tree(the_repository, tree)) {\n> +\t\tret = -1;\n> +\t\tgoto done;\n> +\t}\n> +\n> +\tinit_tree_desc(t, &tree->object.oid, tree->buffer, tree->size);\n> +\n> +\topts.head_idx = 1;\n> +\topts.src_index = the_repository->index;\n> +\topts.dst_index = &dst_istate;\n> +\topts.merge = 1;\n> +\topts.reset = UNPACK_RESET_PROTECT_UNTRACKED;\n> +\topts.fn = oneway_merge;\n> +\n> +\tif (unpack_trees(nr_trees, t, &opts)) {\n> +\t\tret = -1;\n> +\t\tgoto done;\n> +\t}\n> +\n> +\tif (write_locked_index(&dst_istate, &lock_file, COMMIT_LOCK)) {\n> +\t\tret = error(_(\"unable to write new index file\"));\n> +\t\tgoto done;\n> +\t}\n> +\n> +done:\n> +\trelease_index(&dst_istate);\n> +\tif (ret)\n> +\t\trollback_lock_file(&lock_file);\n> +\treturn ret;\n> +}\n> +\n>  static int diff_tree_binary(struct strbuf *out, struct object_id *w_commit)\n>  {\n>  \tstruct child_process cp = CHILD_PROCESS_INIT;\n> @@ -1321,18 +1371,26 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps,\n>  \t\t       struct interactive_options *interactive_opts)\n>  {\n>  \tint ret = 0;\n> -\tstruct child_process cp_read_tree = CHILD_PROCESS_INIT;\n>  \tstruct child_process cp_diff_tree = CHILD_PROCESS_INIT;\n> +\tstruct commit *head_commit;\n> +\tconst struct object_id *head_tree;\n>  \tstruct index_state istate = INDEX_STATE_INIT(the_repository);\n>  \tchar *old_index_env = NULL, *old_repo_index_file;\n>  \n>  \tremove_path(stash_index_path.buf);\n>  \n> -\tcp_read_tree.git_cmd = 1;\n> -\tstrvec_pushl(&cp_read_tree.args, \"read-tree\", \"HEAD\", NULL);\n> -\tstrvec_pushf(&cp_read_tree.env, \"GIT_INDEX_FILE=%s\",\n> -\t\t     stash_index_path.buf);\n> -\tif (run_command(&cp_read_tree)) {\n> +\thead_commit = lookup_commit(the_repository, &info->b_commit);\n> +\tif (!head_commit || repo_parse_commit(the_repository, head_commit)) {\n> +\t\tret = -1;\n> +\t\tgoto done;\n> +\t}\n> +\thead_tree = get_commit_tree_oid(head_commit);\n> +\tif (!head_tree) {\n> +\t\tret = -1;\n> +\t\tgoto done;\n> +\t}\n> +\n> +\tif (create_index_from_tree(head_tree, stash_index_path.buf)) {\n>  \t\tret = -1;\n>  \t\tgoto done;\n>  \t}\n> diff --git a/t/perf/p3904-stash-patch.sh b/t/perf/p3904-stash-patch.sh\n> new file mode 100755\n> index 0000000000..4cfce638be\n> --- /dev/null\n> +++ b/t/perf/p3904-stash-patch.sh\n> @@ -0,0 +1,43 @@\n> +#!/bin/sh\n> +\n> +test_description=\"Performance tests for git stash -p\"\n> +\n> +. ./perf-lib.sh\n> +\n> +test_perf_fresh_repo\n> +\n> +test_expect_success \"setup\" '\n> +\tmkdir files &&\n> +\ttest_seq 1 100000 | while read i; do\n> +\t\techo \"content $i\" >files/$i.txt || return 1\n> +\tdone &&\n> +\tgit add files/ &&\n> +\tgit commit -q -m \"add tracked files\" &&\n> +\techo modified >files/1.txt\n> +'\n> +\n> +test_perf \"stash -p, no fsmonitor\" \\\n> +\t--setup 'echo modified >files/1.txt' '\n> +\tprintf \"q\\n\" | git stash -p >/dev/null 2>&1 || true\n> +'\n> +\n> +if test_have_prereq FSMONITOR_DAEMON\n> +then\n> +\ttest_expect_success \"enable builtin fsmonitor\" '\n> +\t\tgit config core.fsmonitor true &&\n> +\t\tgit fsmonitor--daemon start &&\n> +\t\tgit update-index --fsmonitor &&\n> +\t\tgit status >/dev/null 2>&1\n> +\t'\n> +\n> +\ttest_perf \"stash -p, builtin fsmonitor\" \\\n> +\t\t--setup 'echo modified >files/1.txt && git status >/dev/null 2>&1' '\n> +\t\tprintf \"q\\n\" | git stash -p >/dev/null 2>&1 || true\n> +\t'\n> +\n> +\ttest_expect_success \"stop builtin fsmonitor\" '\n> +\t\tgit fsmonitor--daemon stop\n> +\t'\n> +fi\n> +\n> +test_done\n>\n> base-commit: 7bcaabddcf68bd0702697da5904c3b68c52f94cf\n"}]}