From: Junio C Hamano Date: Wed, 20 May 2026 02:08:38 GMT Subject: Re: [PATCH] stash: reuse cached index entries in --patch temporary index Message-ID: In-Reply-To: "Adam Johnson via GitGitGadget" writes: > From: Adam Johnson > > `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. Clever. As the fsmonitor_valid bit is in-core only, updating the index in-process would be an obvious and probably the only sensible way to preserve it. I however have to wonder if simply replacing the external process invocation with "git read-tree -m HEAD" (i.e., oneway merge) gives a similar speed-up. > 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. Interesting. > diff --git a/t/t3904-stash-patch.sh b/t/t3904-stash-patch.sh > index 90a4ff2c10..4b3241c8cd 100755 > --- a/t/t3904-stash-patch.sh > +++ b/t/t3904-stash-patch.sh > @@ -84,6 +84,24 @@ test_expect_success 'none of this moved HEAD' ' > verify_saved_head > ' > > +test_expect_success 'stash -p with unmodified tracked files present' ' > + git reset --hard && > + echo line1 >alpha && > + echo line1 >beta && > + git add alpha beta && > + git commit -m "add alpha and beta" && > + echo line2 >>alpha && > + echo y | git stash -p && > + echo line1 >expect && > + test_cmp expect alpha && > + test_cmp expect beta && > + git stash pop && > + printf "line1\nline2\n" >expect && > + test_cmp expect alpha && > + echo line1 >expect && > + test_cmp expect beta > +' What I read from the proposed log message is that the change is purely about performance and should not change any behaviour. Why do we need a new test in t/t3904? I would not have surprised if we saw a new test in t/perf/, though. Thanks.