[PATCH v3 0/7] stash creation: avoid unecessary index loads and refreshes

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

Thanks to Junio for his comments on V2, I've fixed a typo in patch
4 and rewritten the commit message for patch 5.

"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.

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%2Fv3
View-Changes-At: https://github.com/phillipwood/git/compare/c46c1e377...a486a625c
Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/stash-optimize-check_changes-calls/v3


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 | 137 ++++++++++++++++++++++--------------------------
 1 file changed, 62 insertions(+), 75 deletions(-)

Range-diff against v2:
1:  9e27bcd676 = 1:  9e27bcd676 stash: use an enum for create status
2:  d56b7c6f67 = 2:  d56b7c6f67 stash create: remove duplicate changes detection
3:  36fc0eb913 = 3:  36fc0eb913 stash push: remove duplicate changes detection
4:  6ca7e1c567 ! 4:  a9875ba5b3 stash push: avoid duplicate calls to preload_index()
    @@ Metadata
      ## Commit message ##
         stash push: avoid duplicate calls to preload_index()
     
    -    do_stash_push() needs to load the index so it can check that any
    +    do_push_stash() needs to load the index so it can check that any
         pathspec argument refers to tracked files. To do that it calls
         repo_read_index_preload() which does not just load the index, it also
         calls preload_index() which lstat()s all the files to see which ones
5:  f799d86f52 ! 5:  de5e0c196a stash create: rework temporary index creation
    @@ Metadata
      ## Commit message ##
         stash create: rework temporary index creation
     
    -    Use create_index_from_tree() instead of reset_tree() when we create a
    -    temporary index file to stash the working tree changes. This matches
    -    what we do for "git stash push -p" and means we can avoid messing with
    -    set_alternate_index_output(). Add a new parameter to control whether
    -    the index is reset, as we need to merge the index rather than reset
    -    it so that we still detect intent-to-add entries. We do not alter the
    -    in-memory index so we can remove the call to repo_read_index_preload()
    -    which saves one lstat() call per changed path.
    +    We use reset_tree() to create the temporary index file used to stash
    +    uncommitted working tree changes. Unfortunately, reset_tree() reads
    +    the tree into the repository's in-memory index and writes the index
    +    to the repository's default index path. In this case, we read the
    +    tree that's already in the index and copy the existing stat data
    +    across, so overwriting the in-memory index should not change its
    +    contents. To write to a temporary index file, the caller has to
    +    use set_alternate_index_output(). While that writes the index to a
    +    temporary file, it updates the timestamp of the in-memory index so
    +    that the timestamp no longer represents the index file the in-memory
    +    index was read from. In practice that probably does not matter, but
    +    set_alternate_index_output() also relies on a global variable which
    +    makes the code harder to reason about.
    +
    +    Use create_index_from_tree() instead of reset_tree() so that we
    +    can create temporary index file without overwriting the in-memory
    +    index or relying on set_alternate_index_output(). This also makes
    +    the implementation consistent with the way we create the temporary
    +    index for "git stash push -p". As create_index_from_tree() does not
    +    modify the in-memory index it is clearly safe to remove the call to
    +    repo_read_index_preload() which saves one lstat() call per changed
    +    path. It would probably be safe to remove that call without moving away
    +    from reset_tree() but it is much easier to reason about now that the
    +    in-memory index is not modified when we create the temporary index file.
    +
    +    In order to use create_index_from_tree() we need to add a new
    +    parameter to control whether the index is reset, as the new caller
    +    needs to merge the index rather than reset it so that we still detect
    +    intent-to-add entries.
     
         Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
     
6:  768d6bc7e5 = 6:  61e4ce22a0 stash create: remove unnecessary index load and refresh
7:  db53bf3b57 = 7:  a486a625c9 stash: simplify reset_tree() function signature
-- 
2.56.0.134.g299a3c16181