Re: [PATCH] stash: reuse prepared state when creating stash

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Jasmeet

On 31/10/2026 20:01, jasmeet.bhatia.us@gmail.com wrote:
> From: "Jasmeet (Jazz) Bhatia" <jasmeet.bhatia.us@gmail.com>
> 
> do_push_stash() already preloads and refreshes the index and calls
> check_changes() before invoking do_create_stash(). However, Git then
> repeats this same index preparation and change detection, including
> rebuilding the list of untracked files when requested, which is
> duplicate work.

Good observation

> Instead, avoid this duplicate work by passing the already prepared
> untracked file list to do_create_stash(). When the list is provided,
> reuse it and skip the redundant index preload, refresh, and
> check_changes() call.

I've got some patches in "seen" [1] that take a slightly different 
approach and keep the call to check_changes() in do_create_stash() and 
remove it from create_stash() and do_push_stash() instead. That series 
also removes a number of unnecessary calls to preload_index(). If you 
have time to post a review that would be really helpful.

Thanks

Phillip

[1] The patches are at 
https://lore.kernel.org/git/cover.1792767794.git.phillip.wood@dunelm.org.uk/ 
and are merged into seen as branch 'pw/stash-change-check-only-once'


> This maintains the existing behavior of "git stash create", which
> invokes do_create_stash() without prepared state and thus continues
> to perform its own index preparation and change detection.
> 
> Testing this using the p3904 stash performance tests shows a decent
> performance boost.
> 
>      stash -p, no fsmonitor
>      0.12(0.11+0.17) -> 0.10(0.10+0.18)  -16.7%
> 
>      stash -p, builtin fsmonitor
>      0.11(0.10+0.02) -> 0.09(0.08+0.01)  -18.2%
> 
> Testing this on the Linux kernel on my system, "git stash push"
> improved from 254 ms to about 239 ms, which is about 6%.
> 
> Signed-off-by: Jasmeet (Jazz) Bhatia <jasmeet.bhatia.us@gmail.com>
> ---
>   builtin/stash.c | 25 +++++++++++++++----------
>   1 file changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/builtin/stash.c b/builtin/stash.c
> index e1a3a76bd0..58eb782597 100644
> --- a/builtin/stash.c
> +++ b/builtin/stash.c
> @@ -1459,7 +1459,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
>   			   int include_untracked, int patch_mode,
>   			   struct interactive_options *interactive_opts,
>   			   int only_staged, struct stash_info *info, struct strbuf *patch,
> -			   int quiet)
> +			   int quiet, struct strbuf *prepared_untracked_files)
>   {
>   	int ret = 0;
>   	int flags = 0;
> @@ -1473,14 +1473,17 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
>   	struct strbuf msg = STRBUF_INIT;
>   	struct strbuf commit_tree_label = STRBUF_INIT;
>   	struct strbuf untracked_files = STRBUF_INIT;
> +	struct strbuf *untracked_files_p = prepared_untracked_files ? prepared_untracked_files : &untracked_files;
>   
>   	prepare_fallback_ident("git stash", "git@stash");
>   
> -	repo_read_index_preload(the_repository, NULL, 0);
> -	if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
> -					 NULL, NULL, NULL) < 0) {
> -		ret = error(_("could not write index"));
> -		goto done;
> +	if (!prepared_untracked_files) {
> +		repo_read_index_preload(the_repository, NULL, 0);
> +		if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
> +						 NULL, NULL, NULL) < 0) {
> +			ret = error(_("could not write index"));
> +			goto done;
> +		}
>   	}
>   
>   	if (repo_get_oid(the_repository, "HEAD", &info->b_commit)) {
> @@ -1493,7 +1496,8 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
>   		head_commit = lookup_commit(the_repository, &info->b_commit);
>   	}
>   
> -	if (!check_changes(ps, include_untracked, &untracked_files)) {
> +	if (!prepared_untracked_files &&
> +	    !check_changes(ps, include_untracked, untracked_files_p)) {
>   		ret = 1;
>   		goto done;
>   	}
> @@ -1529,7 +1533,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
>   	parents = NULL;
>   
>   	if (include_untracked) {
> -		if (save_untracked_files(info, &msg, &untracked_files)) {
> +		if (save_untracked_files(info, &msg, untracked_files_p)) {
>   			if (!quiet)
>   				fprintf_ln(stderr, _("Cannot save "
>   						     "the untracked files"));
> @@ -1615,7 +1619,7 @@ static int create_stash(int argc, const char **argv, const char *prefix UNUSED,
>   		return 0;
>   
>   	ret = do_create_stash(&ps, &stash_msg_buf, 0, 0, NULL, 0, &info,
> -			      NULL, 0);
> +			      NULL, 0, NULL);
>   	if (!ret)
>   		printf_ln("%s", oid_to_hex(&info.w_commit));
>   
> @@ -1698,7 +1702,8 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
>   	if (stash_msg)
>   		strbuf_addstr(&stash_msg_buf, stash_msg);
>   	if (do_create_stash(ps, &stash_msg_buf, include_untracked, patch_mode,
> -			    interactive_opts, only_staged, &info, &patch, quiet)) {
> +			    interactive_opts, only_staged, &info, &patch, quiet,
> +			    &untracked_files)) {
>   		ret = -1;
>   		goto done;
>   	}