[PATCH v3 0/1] unpack-trees: skip stat on fsmonitor-valid files

["Utsav Shah via GitGitGadget" <gitgitgadget@gmail.com>]

The index might be aware that a file hasn't modified via fsmonitor, but
unpack-trees did not pay attention to it and checked via ie_match_stat which
can be inefficient on certain filesystems. This significantly slows down
commands that run oneway_merge, like checkout and reset --hard.

This patch makes oneway_merge check whether a file is considered unchanged
through fsmonitor and skips ie_match_stat on it. unpack-trees also now
correctly copies over fsmonitor validity state from the source index.
Finally, for correctness, we force a refresh of fsmonitor state in
tweak_fsmonitor.

After this change, commands like stash (that use reset --hard internally) go
from 8s or more to ~2s on a 250k file repository on a mac.

Signed-off-by: Utsav Shah utsav@dropbox.com [utsav@dropbox.com]

Utsav Shah (1):
  unpack-trees: skip stat on fsmonitor-valid files

 fsmonitor.c                 | 39 ++++++++++++++++++++++++-------------
 t/t7519-status-fsmonitor.sh |  9 +++++++--
 unpack-trees.c              |  6 +++++-
 3 files changed, 37 insertions(+), 17 deletions(-)


base-commit: 566a1439f6f56c2171b8853ddbca0ad3f5098770
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-424%2FUtsav2%2Fskip-lstat-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-424/Utsav2/skip-lstat-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/424

Range-diff vs v2:

 1:  f76ba554ed ! 1:  4bea7075cf unpack-trees: skip stat on fsmonitor-valid files
     @@ -23,6 +23,15 @@
       --- a/fsmonitor.c
       +++ b/fsmonitor.c
      @@
     + 
     + 	if (pos >= istate->cache_nr)
     + 		BUG("fsmonitor_dirty has more entries than the index (%"PRIuMAX" >= %u)",
     +-		    (uintmax_t)pos, istate->cache_nr);
     ++			(uintmax_t)pos, istate->cache_nr);
     + 
     + 	ce = istate->cache[pos];
     + 	ce->ce_flags &= ~CE_FSMONITOR_VALID;
     +@@
       	}
       	istate->fsmonitor_dirty = fsmonitor_dirty;
       
     @@ -31,7 +40,7 @@
      -		    (uintmax_t)istate->fsmonitor_dirty->bit_size, istate->cache_nr);
      +	if (!istate->split_index && istate->fsmonitor_dirty->bit_size > istate->cache_nr)
      +		BUG("fsmonitor_dirty has more entries than the index (%"PRIuMAX" > %"PRIuMAX")",
     -+		    (uintmax_t)istate->fsmonitor_dirty->bit_size, (uintmax_t)istate->cache_nr);
     ++			(uintmax_t)istate->fsmonitor_dirty->bit_size, (uintmax_t)istate->cache_nr);
      +
       
       	trace_printf_key(&trace_fsmonitor, "read fsmonitor extension successful");
     @@ -45,7 +54,7 @@
      -		    (uintmax_t)istate->fsmonitor_dirty->bit_size, istate->cache_nr);
      +	if (!istate->split_index && istate->fsmonitor_dirty->bit_size > istate->cache_nr)
      +		BUG("fsmonitor_dirty has more entries than the index (%"PRIuMAX" > %"PRIuMAX")",
     -+		    (uintmax_t)istate->fsmonitor_dirty->bit_size, (uintmax_t)istate->cache_nr);
     ++			(uintmax_t)istate->fsmonitor_dirty->bit_size, (uintmax_t)istate->cache_nr);
       
       	put_be32(&hdr_version, INDEX_EXTENSION_VERSION);
       	strbuf_add(sb, &hdr_version, sizeof(uint32_t));
     @@ -57,10 +66,33 @@
      +		if (istate->untracked)
      +			istate->untracked->use_fsmonitor = 1;
       	} else {
     ++
     ++		/* We only want to run the post index changed hook if we've actually changed entries, so keep track
     ++		 * if we actually changed entries or not */
     ++		int is_cache_changed = 0;
       		/* Mark all entries invalid */
     - 		for (i = 0; i < istate->cache_nr; i++)
     +-		for (i = 0; i < istate->cache_nr; i++)
     +-			istate->cache[i]->ce_flags &= ~CE_FSMONITOR_VALID;
     ++		for (i = 0; i < istate->cache_nr; i++) {
     ++			if (istate->cache[i]->ce_flags & CE_FSMONITOR_VALID) {
     ++				is_cache_changed = 1;
     ++				istate->cache[i]->ce_flags &= ~CE_FSMONITOR_VALID;
     ++			}
     ++		}
     + 
     + 		/* If we're going to check every file, ensure we save the results */
     +-		istate->cache_changed |= FSMONITOR_CHANGED;
     ++		if (is_cache_changed)
     ++			istate->cache_changed |= FSMONITOR_CHANGED;
     + 
     + 		if (istate->untracked)
     + 			istate->untracked->use_fsmonitor = 0;
      @@
     - 				    (uintmax_t)istate->fsmonitor_dirty->bit_size, istate->cache_nr);
     + 			/* Mark all previously saved entries as dirty */
     + 			if (istate->fsmonitor_dirty->bit_size > istate->cache_nr)
     + 				BUG("fsmonitor_dirty has more entries than the index (%"PRIuMAX" > %u)",
     +-				    (uintmax_t)istate->fsmonitor_dirty->bit_size, istate->cache_nr);
     ++					(uintmax_t)istate->fsmonitor_dirty->bit_size, istate->cache_nr);
       			ewah_each_bit(istate->fsmonitor_dirty, fsmonitor_ewah_callback, istate);
       
      -			/* Now mark the untracked cache for fsmonitor usage */
     @@ -71,20 +103,6 @@
       
       		ewah_free(istate->fsmonitor_dirty);
      
     - diff --git a/t/t7113-post-index-change-hook.sh b/t/t7113-post-index-change-hook.sh
     - --- a/t/t7113-post-index-change-hook.sh
     - +++ b/t/t7113-post-index-change-hook.sh
     -@@
     - 	git checkout -- dir1/file1.txt &&
     - 	test_path_is_file testsuccess && rm -f testsuccess &&
     - 	test_path_is_missing testfailure &&
     --	git update-index &&
     --	test_path_is_missing testsuccess &&
     --	test_path_is_missing testfailure &&
     - 	git reset --soft &&
     - 	test_path_is_missing testsuccess &&
     - 	test_path_is_missing testfailure
     -
       diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
       --- a/t/t7519-status-fsmonitor.sh
       +++ b/t/t7519-status-fsmonitor.sh
     @@ -113,8 +131,8 @@
      -	write_script .git/hooks/fsmonitor-test<<-\EOF &&
      -	EOF
       	clean_repo &&
     -+	write_integration_script &&
       	dirty_repo &&
     ++	write_integration_script &&
       	git add . &&
      +	write_script .git/hooks/fsmonitor-test<<-\EOF &&
      +	EOF

-- 
gitgitgadget