* [PATCH] stash: reuse prepared state when creating stash
@ 2026-10-31 20:01 jasmeet.bhatia.us
2026-11-02 14:14 ` Phillip Wood
0 siblings, 1 reply; 3+ messages in thread
From: jasmeet.bhatia.us @ 2026-10-31 20:01 UTC (permalink / raw)
To: git
Cc: Jasmeet (Jazz) Bhatia, Deveshi Dwivedi, Junio C Hamano,
Ævar Arnfjörð Bjarmason, Patrick Steinhardt,
Sergey Organov, Victoria Dye, Leon Michalak
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.
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.
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;
}
--
2.56.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] stash: reuse prepared state when creating stash
2026-10-31 20:01 [PATCH] stash: reuse prepared state when creating stash jasmeet.bhatia.us
@ 2026-11-02 14:14 ` Phillip Wood
2026-11-09 5:09 ` jasmeet.bhatia.us
0 siblings, 1 reply; 3+ messages in thread
From: Phillip Wood @ 2026-11-02 14:14 UTC (permalink / raw)
To: jasmeet.bhatia.us, git
Cc: Deveshi Dwivedi, Junio C Hamano, Patrick Steinhardt,
Leon Michalak
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;
> }
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] stash: reuse prepared state when creating stash
2026-11-02 14:14 ` Phillip Wood
@ 2026-11-09 5:09 ` jasmeet.bhatia.us
0 siblings, 0 replies; 3+ messages in thread
From: jasmeet.bhatia.us @ 2026-11-09 5:09 UTC (permalink / raw)
To: phillip.wood; +Cc: git, deveshigurgaon, gitster, ps, leonmichalak6
Hi Phillip,
On Mon Nov 2, 2026 at 6:14 AM -08:00, Phillip Wood wrote:
> 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 for the opportunity to contribute! I'll take a look in the coming days and let you know any thoughts I have.
Thanks,
Jazz
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-11-09 5:09 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-31 20:01 [PATCH] stash: reuse prepared state when creating stash jasmeet.bhatia.us
2026-11-02 14:14 ` Phillip Wood
2026-11-09 5:09 ` jasmeet.bhatia.us
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox