Re: [PATCH v2] stash: reuse cached index entries in --patch temporary index
From: Junio C Hamano <hidden>
Date: 2026-06-01 21:33:15
"Adam Johnson via GitGitGadget" [off-list ref] writes:
From: Adam Johnson <redacted>
`git stash -p` prepares the interactive selection by creating a
temporary index at HEAD, switching `GIT_INDEX_FILE` to it, and then
running the `add -p` machinery.
That temporary index was created by running `git read-tree HEAD`. The
resulting index had no useful cached stat data or fsmonitor-valid bits
from the real index. When `run_add_p()` refreshed that temporary index
before showing the first prompt, it could end up lstat(2)-ing every
tracked file, even in a repository where `git diff` and `git restore -p`
can use fsmonitor to avoid that work.
Create the temporary index in-process instead. Use `unpack_trees()` to
reset the real index contents to HEAD while writing the result to the
temporary index path. For paths whose index entries already match HEAD,
`oneway_merge()` reuses the existing cache entries, preserving their
cached stat data and `CE_FSMONITOR_VALID` state.
This makes the refresh performed by `run_add_p()` behave like the one
used by `git restore -p`: unchanged paths can be skipped via fsmonitor
instead of being scanned again.
In a 206k file repository with `core.fsmonitor` enabled and a one-line
change in one file, time to first prompt dropped from 34.774 seconds to
0.659 seconds. The new perf test file demonstrates similar improvements,
with maen times for without- and with-fsmonitor cases dropping from 6.90
and 6.83 seconds to 0.55 and 0.28 seconds, respectively.
Signed-off-by: Adam Johnson <redacted>
---
stash: reuse cached index entries in --patch temporary index
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2306%2Fadamchainz%2Faj%2Foptimize-stash-patch-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2306/adamchainz/aj/optimize-stash-patch-v2
Pull-Request: https://github.com/git/git/pull/2306The diff relative to the previous round looked good. I am not a "stash -p" user myself, but I suspect that there are people who heavily use it, so I'd feel safer if an extra set of eye looks at the patch and gives an Ack, but other than that I have no comments on the patch. Looking good. Thanks.
quoted hunk ↗ jump to hunk
builtin/stash.c | 70 +++++++++++++++++++++++++++++++++---- t/perf/p3904-stash-patch.sh | 43 +++++++++++++++++++++++ 2 files changed, 107 insertions(+), 6 deletions(-) create mode 100755 t/perf/p3904-stash-patch.shdiff --git a/builtin/stash.c b/builtin/stash.c index 32dbc97b47..c4809f299a 100644 --- a/builtin/stash.c +++ b/builtin/stash.c@@ -372,6 +372,56 @@ static int reset_tree(struct object_id *i_tree, int update, int reset) return 0; } +static int create_index_from_tree(const struct object_id *tree_id, + const char *index_path) +{ + int nr_trees = 1; + int ret = 0; + struct unpack_trees_options opts; + struct tree_desc t[MAX_UNPACK_TREES]; + struct tree *tree; + struct index_state dst_istate = INDEX_STATE_INIT(the_repository); + struct lock_file lock_file = LOCK_INIT; + + repo_read_index_preload(the_repository, NULL, 0); + refresh_index(the_repository->index, REFRESH_QUIET, NULL, NULL, NULL); + + hold_lock_file_for_update(&lock_file, index_path, LOCK_DIE_ON_ERROR); + + memset(&opts, 0, sizeof(opts)); + + tree = repo_parse_tree_indirect(the_repository, tree_id); + if (!tree || repo_parse_tree(the_repository, tree)) { + ret = -1; + goto done; + } + + init_tree_desc(t, &tree->object.oid, tree->buffer, tree->size); + + opts.head_idx = 1; + opts.src_index = the_repository->index; + opts.dst_index = &dst_istate; + opts.merge = 1; + opts.reset = UNPACK_RESET_PROTECT_UNTRACKED; + opts.fn = oneway_merge; + + if (unpack_trees(nr_trees, t, &opts)) { + ret = -1; + goto done; + } + + if (write_locked_index(&dst_istate, &lock_file, COMMIT_LOCK)) { + ret = error(_("unable to write new index file")); + goto done; + } + +done: + release_index(&dst_istate); + if (ret) + rollback_lock_file(&lock_file); + return ret; +} + static int diff_tree_binary(struct strbuf *out, struct object_id *w_commit) { struct child_process cp = CHILD_PROCESS_INIT;@@ -1321,18 +1371,26 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps, struct interactive_options *interactive_opts) { int ret = 0; - struct child_process cp_read_tree = CHILD_PROCESS_INIT; struct child_process cp_diff_tree = CHILD_PROCESS_INIT; + struct commit *head_commit; + const struct object_id *head_tree; struct index_state istate = INDEX_STATE_INIT(the_repository); char *old_index_env = NULL, *old_repo_index_file; remove_path(stash_index_path.buf); - cp_read_tree.git_cmd = 1; - strvec_pushl(&cp_read_tree.args, "read-tree", "HEAD", NULL); - strvec_pushf(&cp_read_tree.env, "GIT_INDEX_FILE=%s", - stash_index_path.buf); - if (run_command(&cp_read_tree)) { + head_commit = lookup_commit(the_repository, &info->b_commit); + if (!head_commit || repo_parse_commit(the_repository, head_commit)) { + ret = -1; + goto done; + } + head_tree = get_commit_tree_oid(head_commit); + if (!head_tree) { + ret = -1; + goto done; + } + + if (create_index_from_tree(head_tree, stash_index_path.buf)) { ret = -1; goto done; }diff --git a/t/perf/p3904-stash-patch.sh b/t/perf/p3904-stash-patch.sh new file mode 100755 index 0000000000..4cfce638be --- /dev/null +++ b/t/perf/p3904-stash-patch.sh@@ -0,0 +1,43 @@ +#!/bin/sh + +test_description="Performance tests for git stash -p" + +. ./perf-lib.sh + +test_perf_fresh_repo + +test_expect_success "setup" ' + mkdir files && + test_seq 1 100000 | while read i; do + echo "content $i" >files/$i.txt || return 1 + done && + git add files/ && + git commit -q -m "add tracked files" && + echo modified >files/1.txt +' + +test_perf "stash -p, no fsmonitor" \ + --setup 'echo modified >files/1.txt' ' + printf "q\n" | git stash -p >/dev/null 2>&1 || true +' + +if test_have_prereq FSMONITOR_DAEMON +then + test_expect_success "enable builtin fsmonitor" ' + git config core.fsmonitor true && + git fsmonitor--daemon start && + git update-index --fsmonitor && + git status >/dev/null 2>&1 + ' + + test_perf "stash -p, builtin fsmonitor" \ + --setup 'echo modified >files/1.txt && git status >/dev/null 2>&1' ' + printf "q\n" | git stash -p >/dev/null 2>&1 || true + ' + + test_expect_success "stop builtin fsmonitor" ' + git fsmonitor--daemon stop + ' +fi + +test_donebase-commit: 7bcaabddcf68bd0702697da5904c3b68c52f94cf