Git development
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: git@vger.kernel.org
Cc: 重田一聖 <kazumasa.shigeta@kanamei.com>,
	"Phillip Wood" <phillip.wood123@gmail.com>
Subject: [PATCH v4 0/7] stash creation: avoid unecessary index loads and refreshes
Date: Fri, 23 Oct 2026 16:03:28 +0100	[thread overview]
Message-ID: <cover.1792767794.git.phillip.wood@dunelm.org.uk> (raw)
In-Reply-To: <cover.1791218125.git.phillip.wood@dunelm.org.uk>

"git stash push" and "git stash create" load and refresh the index
multiple times when they only need to do it once. This series removes
a number of calls to repo_read_preload_index() and refresh_index()
which reduces the number of lstat() calls on each changed path by
four for "git stash create" and eight for "git stash push". It also
removes a duplicate call to check_changes() so that when stashing
untracked files we only walk them once.

Thanks to Junio for his comments on V3

Changes since V3:
 - Avoid implict cast of enum create_result to int in create_stash()
 - Fixed enum initializers
 - Use switch() rather than if() when checking enum create_result
   return values

Changes since V2:
 - Fixed a typo in patch 4
 - Rewrote commit message for patch 5

Changes since V1:
 - Added an enum for the various return values from do_create_stash()
 - Fixed typos in the commit message of patch 2 (was patch 1)
 - Removed an unnecessary index refresh and several unnecessary index
   preloads

Cover letter for V1:

"git stash push" and "git stash create" check if there are
any unstaged or uncommitted changes at startup, and then again
when they try to create the stash. This short series removes
that duplication of effort which I spotted while looking at
<20260929074222.11942-1-kazumasa.shigeta@kanamei.com>.

base-commit: c46c1e37724f0478939de636ab8ea5a89086d532
Published-As: https://github.com/phillipwood/git/releases/tag/pw%2Fstash-optimize-check_changes-calls%2Fv4
View-Changes-At: https://github.com/phillipwood/git/compare/c46c1e377...f61ebbf9a
Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/stash-optimize-check_changes-calls/v4


Phillip Wood (7):
  stash: use an enum for create status
  stash create: remove duplicate changes detection
  stash push: remove duplicate changes detection
  stash push: avoid duplicate calls to preload_index()
  stash create: rework temporary index creation
  stash create: remove unnecessary index load and refresh
  stash: simplify reset_tree() function signature

 builtin/stash.c | 167 ++++++++++++++++++++++++------------------------
 1 file changed, 85 insertions(+), 82 deletions(-)

Range-diff against v3:
1:  9e27bcd676e ! 1:  685914011f9 stash: use an enum for create status
    @@ builtin/stash.c: static int save_untracked_files(struct stash_info *info, struct
     +				  struct strbuf *out_patch, int quiet)
      {
     -	int ret = 0;
    -+	enum create_result ret = 0;
    ++	enum create_result ret = CREATE_SUCCESS;
      	struct child_process cp_diff_tree = CHILD_PROCESS_INIT;
      	struct index_state istate = INDEX_STATE_INIT(the_repository);
      
    @@ builtin/stash.c: static int stash_staged(struct stash_info *info, struct strbuf
     +		struct interactive_options *interactive_opts)
      {
     -	int ret = 0;
    -+	enum create_result ret = 0;
    ++	enum create_result ret = CREATE_SUCCESS;
      	struct child_process cp_diff_tree = CHILD_PROCESS_INIT;
      	struct commit *head_commit;
      	const struct object_id *head_tree;
    @@ builtin/stash.c: static int stash_working_tree(struct stash_info *info, const st
     +		int quiet)
      {
     -	int ret = 0;
    -+	enum create_result ret = 0;
    ++	enum create_result ret = CREATE_SUCCESS;
      	int flags = 0;
      	int untracked_commit_option = 0;
      	const char *head_short_sha1 = NULL;
    @@ builtin/stash.c: static int do_create_stash(const struct pathspec *ps, struct st
      	if (patch_mode) {
      		ret = stash_patch(info, ps, patch, quiet, interactive_opts);
     -		if (ret < 0) {
    -+		if (ret == CREATE_ERROR) {
    ++		switch (ret) {
    ++		case CREATE_ERROR:
      			if (!quiet)
      				fprintf_ln(stderr, _("Cannot save the current "
      						     "worktree state"));
    - 			goto done;
    +-			goto done;
     -		} else if (ret > 0) {
    -+		} else if (ret != CREATE_SUCCESS) {
    - 			goto done;
    +-			goto done;
    ++			/* fallthrough */
    ++		case CREATE_EMPTY:
    ++			goto done;
    ++
    ++		case CREATE_SUCCESS:
    ++			break;
      		}
      	} else if (only_staged) {
      		ret = stash_staged(info, patch, quiet);
     -		if (ret < 0) {
    -+		if (ret == CREATE_ERROR) {
    ++		switch (ret) {
    ++			case CREATE_ERROR:
      			if (!quiet)
      				fprintf_ln(stderr, _("Cannot save the current "
      						     "staged state"));
    - 			goto done;
    +-			goto done;
     -		} else if (ret > 0) {
    -+		} else if (ret != CREATE_SUCCESS) {
    - 			goto done;
    +-			goto done;
    ++			/* fallthrough */
    ++			case CREATE_EMPTY:
    ++				goto done;
    ++
    ++			case CREATE_SUCCESS:
    ++				break;
      		}
      	} else {
      		if (stash_working_tree(info, ps)) {
    @@ builtin/stash.c: static int do_create_stash(const struct pathspec *ps, struct st
      		goto done;
      	}
      
    +@@ builtin/stash.c: static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
    + static int create_stash(int argc, const char **argv, const char *prefix UNUSED,
    + 			struct repository *repo UNUSED)
    + {
    +-	int ret;
    ++	enum create_result create_res;
    + 	struct strbuf stash_msg_buf = STRBUF_INIT;
    + 	struct stash_info info = STASH_INFO_INIT;
    + 	struct pathspec ps;
     @@ builtin/stash.c: static int create_stash(int argc, const char **argv, const char *prefix UNUSED,
    + 	if (!check_changes_tracked_files(&ps))
    + 		return 0;
      
    - 	ret = do_create_stash(&ps, &stash_msg_buf, 0, 0, NULL, 0, &info,
    - 			      NULL, 0);
    +-	ret = do_create_stash(&ps, &stash_msg_buf, 0, 0, NULL, 0, &info,
    +-			      NULL, 0);
     -	if (!ret)
    -+	if (ret == CREATE_SUCCESS)
    ++	create_res = do_create_stash(&ps, &stash_msg_buf, 0, 0, NULL, 0, &info,
    ++				     NULL, 0);
    ++	if (create_res == CREATE_SUCCESS)
      		printf_ln("%s", oid_to_hex(&info.w_commit));
      
      	free_stash_info(&info);
      	strbuf_release(&stash_msg_buf);
     -	return ret;
     +	/* Do not return an error if there are no changes */
    -+	return ret == CREATE_ERROR ? -1 : 0;
    ++	return create_res == CREATE_ERROR ? -1 : 0;
      }
      
      static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int quiet,
    @@ builtin/stash.c: static int do_push_stash(const struct pathspec *ps, const char
     +	create_res =  do_create_stash(ps, &stash_msg_buf, include_untracked,
     +				      patch_mode, interactive_opts, only_staged,
     +				      &info, &patch, quiet);
    -+	if (create_res != CREATE_SUCCESS) {
    ++	switch (create_res) {
    ++	case CREATE_ERROR:
    ++	case CREATE_EMPTY:
      		ret = -1;
      		goto done;
    ++
    ++	case CREATE_SUCCESS:
    ++		break;
      	}
    + 
    + 	if (do_store_stash(&info.w_commit, stash_msg_buf.buf, 1)) {
2:  d56b7c6f672 ! 2:  d2773fc82eb stash create: remove duplicate changes detection
    @@ builtin/stash.c: static int create_stash(int argc, const char **argv, const char
     -	if (!check_changes_tracked_files(&ps))
     -		return 0;
      
    - 	ret = do_create_stash(&ps, &stash_msg_buf, 0, 0, NULL, 0, &info,
    - 			      NULL, 0);
    + 	create_res = do_create_stash(&ps, &stash_msg_buf, 0, 0, NULL, 0, &info,
    + 				     NULL, 0);
3:  36fc0eb9130 ! 3:  785d2a663da stash push: remove duplicate changes detection
    @@ builtin/stash.c: static enum create_result do_create_stash(const struct pathspec
      		goto done;
      	}
      
    +@@ builtin/stash.c: static enum create_result do_create_stash(const struct pathspec *ps,
    + 						     "worktree state"));
    + 			/* fallthrough */
    + 		case CREATE_EMPTY:
    ++		case CREATE_NO_CHANGES:
    + 			goto done;
    + 
    + 		case CREATE_SUCCESS:
    +@@ builtin/stash.c: static enum create_result do_create_stash(const struct pathspec *ps,
    + 						     "staged state"));
    + 			/* fallthrough */
    + 			case CREATE_EMPTY:
    ++			case CREATE_NO_CHANGES:
    + 				goto done;
    + 
    + 			case CREATE_SUCCESS:
     @@ builtin/stash.c: static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
      		free(ps_matched);
      	}
    @@ builtin/stash.c: static int do_push_stash(const struct pathspec *ps, const char
      		ret = -1;
      		if (!quiet)
     @@ builtin/stash.c: static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
    - 	create_res =  do_create_stash(ps, &stash_msg_buf, include_untracked,
      				      patch_mode, interactive_opts, only_staged,
      				      &info, &patch, quiet);
    --	if (create_res != CREATE_SUCCESS) {
    -+	if (create_res == CREATE_NO_CHANGES) {
    + 	switch (create_res) {
    ++	case CREATE_NO_CHANGES:
     +		if (!quiet)
     +			printf_ln(_("No local changes to save"));
     +		ret = 0;
     +		goto done;
    -+	} else if (create_res != CREATE_SUCCESS) {
    ++
    + 	case CREATE_ERROR:
    + 	case CREATE_EMPTY:
      		ret = -1;
    - 		goto done;
    - 	}
4:  a9875ba5b35 ! 4:  32f7c97b7c0 stash push: avoid duplicate calls to preload_index()
    @@ builtin/stash.c: static int create_stash(int argc, const char **argv, const char
      	memset(&ps, 0, sizeof(ps));
      
     +	repo_read_index(the_repository);
    - 	ret = do_create_stash(&ps, &stash_msg_buf, 0, 0, NULL, 0, &info,
    - 			      NULL, 0);
    - 	if (ret == CREATE_SUCCESS)
    + 	create_res = do_create_stash(&ps, &stash_msg_buf, 0, 0, NULL, 0, &info,
    + 				     NULL, 0);
    + 	if (create_res == CREATE_SUCCESS)
     @@ builtin/stash.c: static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
      		goto done;
      	}
5:  de5e0c196a9 = 5:  b62d3d080a8 stash create: rework temporary index creation
6:  61e4ce22a0f = 6:  8c54c118307 stash create: remove unnecessary index load and refresh
7:  a486a625c95 = 7:  f61ebbf9a3e stash: simplify reset_tree() function signature
-- 
2.56.0.134.g299a3c16181


  parent reply	other threads:[~2026-10-23 15:03 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-05 16:35 [PATCH 0/2] stash: stop checking for changes twice Phillip Wood
2026-10-05 16:35 ` [PATCH 1/2] stash create: remove duplicate changes detection Phillip Wood
2026-10-06 12:44   ` Junio C Hamano
2026-10-07 13:49     ` Phillip Wood
2026-10-05 16:35 ` [PATCH 2/2] stash push: " Phillip Wood
2026-10-06 14:27   ` Junio C Hamano
2026-10-07 13:46     ` Phillip Wood
2026-10-07 17:15       ` Junio C Hamano
2026-10-08 13:35         ` Phillip Wood
2026-10-14 15:40 ` [PATCH v2 0/7] stash creation: avoid unecessary index loads and refreshes Phillip Wood
2026-10-14 15:40   ` [PATCH v2 1/7] stash: use an enum for create status Phillip Wood
2026-10-14 15:40   ` [PATCH v2 2/7] stash create: remove duplicate changes detection Phillip Wood
2026-10-14 15:40   ` [PATCH v2 3/7] stash push: " Phillip Wood
2026-10-14 15:40   ` [PATCH v2 4/7] stash push: avoid duplicate calls to preload_index() Phillip Wood
2026-10-14 18:09     ` Junio C Hamano
2026-10-14 15:40   ` [PATCH v2 5/7] stash create: rework temporary index creation Phillip Wood
2026-10-14 18:16     ` Junio C Hamano
2026-10-20 14:54       ` Phillip Wood
2026-10-14 15:40   ` [PATCH v2 6/7] stash create: remove unnecessary index load and refresh Phillip Wood
2026-10-14 18:21     ` Junio C Hamano
2026-10-14 15:40   ` [PATCH v2 7/7] stash: simplify reset_tree() function signature Phillip Wood
2026-10-20 15:20 ` [PATCH v3 0/7] stash creation: avoid unecessary index loads and refreshes Phillip Wood
2026-10-20 15:20   ` [PATCH v3 1/7] stash: use an enum for create status Phillip Wood
2026-10-21  6:07     ` Junio C Hamano
2026-10-22 15:57       ` Phillip Wood
2026-10-22 17:01         ` Junio C Hamano
2026-10-22 18:33           ` Phillip Wood
2026-10-20 15:20   ` [PATCH v3 2/7] stash create: remove duplicate changes detection Phillip Wood
2026-10-20 15:20   ` [PATCH v3 3/7] stash push: " Phillip Wood
2026-10-20 15:20   ` [PATCH v3 4/7] stash push: avoid duplicate calls to preload_index() Phillip Wood
2026-10-20 15:20   ` [PATCH v3 5/7] stash create: rework temporary index creation Phillip Wood
2026-10-20 22:07     ` Junio C Hamano
2026-10-20 15:20   ` [PATCH v3 6/7] stash create: remove unnecessary index load and refresh Phillip Wood
2026-10-20 15:20   ` [PATCH v3 7/7] stash: simplify reset_tree() function signature Phillip Wood
2026-10-21  0:07   ` [PATCH v3 0/7] stash creation: avoid unecessary index loads and refreshes Junio C Hamano
2026-10-23 15:03 ` Phillip Wood [this message]
2026-10-23 15:03   ` [PATCH v4 1/7] stash: use an enum for create status Phillip Wood
2026-10-23 15:03   ` [PATCH v4 2/7] stash create: remove duplicate changes detection Phillip Wood
2026-10-23 15:03   ` [PATCH v4 3/7] stash push: " Phillip Wood
2026-10-23 15:03   ` [PATCH v4 4/7] stash push: avoid duplicate calls to preload_index() Phillip Wood
2026-10-23 15:03   ` [PATCH v4 5/7] stash create: rework temporary index creation Phillip Wood
2026-10-23 15:03   ` [PATCH v4 6/7] stash create: remove unnecessary index load and refresh Phillip Wood
2026-10-23 15:03   ` [PATCH v4 7/7] stash: simplify reset_tree() function signature Phillip Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1792767794.git.phillip.wood@dunelm.org.uk \
    --to=phillip.wood123@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=kazumasa.shigeta@kanamei.com \
    --cc=phillip.wood@dunelm.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox