Merge branch 'tk/untracked-cache-with-uall'
The performance of the "untracked cache" feature has been improved when "--untracked-files=<mode>" and "status.showUntrackedFiles" are combined. * tk/untracked-cache-with-uall: untracked-cache: support '--untracked-files=all' if configured untracked-cache: test untracked-cache-bypassing behavior with -uall
This commit is contained in:
commit
301fc17de0
88
dir.c
88
dir.c
@ -2747,13 +2747,33 @@ static void set_untracked_ident(struct untracked_cache *uc)
|
||||
strbuf_addch(&uc->ident, 0);
|
||||
}
|
||||
|
||||
static void new_untracked_cache(struct index_state *istate)
|
||||
static unsigned new_untracked_cache_flags(struct index_state *istate)
|
||||
{
|
||||
struct repository *repo = istate->repo;
|
||||
char *val;
|
||||
|
||||
/*
|
||||
* This logic is coordinated with the setting of these flags in
|
||||
* wt-status.c#wt_status_collect_untracked(), and the evaluation
|
||||
* of the config setting in commit.c#git_status_config()
|
||||
*/
|
||||
if (!repo_config_get_string(repo, "status.showuntrackedfiles", &val) &&
|
||||
!strcmp(val, "all"))
|
||||
return 0;
|
||||
|
||||
/*
|
||||
* The default, if "all" is not set, is "normal" - leading us here.
|
||||
* If the value is "none" then it really doesn't matter.
|
||||
*/
|
||||
return DIR_SHOW_OTHER_DIRECTORIES | DIR_HIDE_EMPTY_DIRECTORIES;
|
||||
}
|
||||
|
||||
static void new_untracked_cache(struct index_state *istate, int flags)
|
||||
{
|
||||
struct untracked_cache *uc = xcalloc(1, sizeof(*uc));
|
||||
strbuf_init(&uc->ident, 100);
|
||||
uc->exclude_per_dir = ".gitignore";
|
||||
/* should be the same flags used by git-status */
|
||||
uc->dir_flags = DIR_SHOW_OTHER_DIRECTORIES | DIR_HIDE_EMPTY_DIRECTORIES;
|
||||
uc->dir_flags = flags >= 0 ? flags : new_untracked_cache_flags(istate);
|
||||
set_untracked_ident(uc);
|
||||
istate->untracked = uc;
|
||||
istate->cache_changed |= UNTRACKED_CHANGED;
|
||||
@ -2762,11 +2782,11 @@ static void new_untracked_cache(struct index_state *istate)
|
||||
void add_untracked_cache(struct index_state *istate)
|
||||
{
|
||||
if (!istate->untracked) {
|
||||
new_untracked_cache(istate);
|
||||
new_untracked_cache(istate, -1);
|
||||
} else {
|
||||
if (!ident_in_untracked(istate->untracked)) {
|
||||
free_untracked_cache(istate->untracked);
|
||||
new_untracked_cache(istate);
|
||||
new_untracked_cache(istate, -1);
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -2814,17 +2834,9 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *d
|
||||
if (base_len || (pathspec && pathspec->nr))
|
||||
return NULL;
|
||||
|
||||
/* Different set of flags may produce different results */
|
||||
if (dir->flags != dir->untracked->dir_flags ||
|
||||
/*
|
||||
* See treat_directory(), case index_nonexistent. Without
|
||||
* this flag, we may need to also cache .git file content
|
||||
* for the resolve_gitlink_ref() call, which we don't.
|
||||
*/
|
||||
!(dir->flags & DIR_SHOW_OTHER_DIRECTORIES) ||
|
||||
/* We don't support collecting ignore files */
|
||||
(dir->flags & (DIR_SHOW_IGNORED | DIR_SHOW_IGNORED_TOO |
|
||||
DIR_COLLECT_IGNORED)))
|
||||
/* We don't support collecting ignore files */
|
||||
if (dir->flags & (DIR_SHOW_IGNORED | DIR_SHOW_IGNORED_TOO |
|
||||
DIR_COLLECT_IGNORED))
|
||||
return NULL;
|
||||
|
||||
/*
|
||||
@ -2847,6 +2859,50 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *d
|
||||
return NULL;
|
||||
}
|
||||
|
||||
/*
|
||||
* If the untracked structure we received does not have the same flags
|
||||
* as requested in this run, we're going to need to either discard the
|
||||
* existing structure (and potentially later recreate), or bypass the
|
||||
* untracked cache mechanism for this run.
|
||||
*/
|
||||
if (dir->flags != dir->untracked->dir_flags) {
|
||||
/*
|
||||
* If the untracked structure we received does not have the same flags
|
||||
* as configured, then we need to reset / create a new "untracked"
|
||||
* structure to match the new config.
|
||||
*
|
||||
* Keeping the saved and used untracked cache consistent with the
|
||||
* configuration provides an opportunity for frequent users of
|
||||
* "git status -uall" to leverage the untracked cache by aligning their
|
||||
* configuration - setting "status.showuntrackedfiles" to "all" or
|
||||
* "normal" as appropriate.
|
||||
*
|
||||
* Previously using -uall (or setting "status.showuntrackedfiles" to
|
||||
* "all") was incompatible with untracked cache and *consistently*
|
||||
* caused surprisingly bad performance (with fscache and fsmonitor
|
||||
* enabled) on Windows.
|
||||
*
|
||||
* IMPROVEMENT OPPORTUNITY: If we reworked the untracked cache storage
|
||||
* to not be as bound up with the desired output in a given run,
|
||||
* and instead iterated through and stored enough information to
|
||||
* correctly serve both "modes", then users could get peak performance
|
||||
* with or without '-uall' regardless of their
|
||||
* "status.showuntrackedfiles" config.
|
||||
*/
|
||||
if (dir->untracked->dir_flags != new_untracked_cache_flags(istate)) {
|
||||
free_untracked_cache(istate->untracked);
|
||||
new_untracked_cache(istate, dir->flags);
|
||||
dir->untracked = istate->untracked;
|
||||
}
|
||||
else {
|
||||
/*
|
||||
* Current untracked cache data is consistent with config, but not
|
||||
* usable in this request/run; just bypass untracked cache.
|
||||
*/
|
||||
return NULL;
|
||||
}
|
||||
}
|
||||
|
||||
if (!dir->untracked->root) {
|
||||
/* Untracked cache existed but is not initialized; fix that */
|
||||
FLEX_ALLOC_STR(dir->untracked->root, name, "");
|
||||
|
@ -190,6 +190,119 @@ test_expect_success 'untracked cache after second status' '
|
||||
test_cmp ../dump.expect ../actual
|
||||
'
|
||||
|
||||
cat >../status_uall.expect <<EOF &&
|
||||
A done/one
|
||||
A one
|
||||
A two
|
||||
?? dthree/three
|
||||
?? dtwo/two
|
||||
?? three
|
||||
EOF
|
||||
|
||||
# Bypassing the untracked cache here is not desirable from an
|
||||
# end-user perspective, but is expected in the current design.
|
||||
# The untracked cache data stored for a -unormal run cannot be
|
||||
# correctly used in a -uall run - it would yield incorrect output.
|
||||
test_expect_success 'untracked cache is bypassed with -uall' '
|
||||
: >../trace.output &&
|
||||
GIT_TRACE2_PERF="$TRASH_DIRECTORY/trace.output" \
|
||||
git status -uall --porcelain >../actual &&
|
||||
iuc status -uall --porcelain >../status.iuc &&
|
||||
test_cmp ../status_uall.expect ../status.iuc &&
|
||||
test_cmp ../status_uall.expect ../actual &&
|
||||
get_relevant_traces ../trace.output ../trace.relevant &&
|
||||
cat >../trace.expect <<EOF &&
|
||||
....path:
|
||||
EOF
|
||||
test_cmp ../trace.expect ../trace.relevant
|
||||
'
|
||||
|
||||
test_expect_success 'untracked cache remains after bypass' '
|
||||
test-tool dump-untracked-cache >../actual &&
|
||||
test_cmp ../dump.expect ../actual
|
||||
'
|
||||
|
||||
test_expect_success 'if -uall is configured, untracked cache gets populated by default' '
|
||||
test_config status.showuntrackedfiles all &&
|
||||
: >../trace.output &&
|
||||
GIT_TRACE2_PERF="$TRASH_DIRECTORY/trace.output" \
|
||||
git status --porcelain >../actual &&
|
||||
iuc status --porcelain >../status.iuc &&
|
||||
test_cmp ../status_uall.expect ../status.iuc &&
|
||||
test_cmp ../status_uall.expect ../actual &&
|
||||
get_relevant_traces ../trace.output ../trace.relevant &&
|
||||
cat >../trace.expect <<EOF &&
|
||||
....path:
|
||||
....node-creation:3
|
||||
....gitignore-invalidation:1
|
||||
....directory-invalidation:0
|
||||
....opendir:4
|
||||
EOF
|
||||
test_cmp ../trace.expect ../trace.relevant
|
||||
'
|
||||
|
||||
cat >../dump_uall.expect <<EOF &&
|
||||
info/exclude $EMPTY_BLOB
|
||||
core.excludesfile $ZERO_OID
|
||||
exclude_per_dir .gitignore
|
||||
flags 00000000
|
||||
/ $ZERO_OID recurse valid
|
||||
three
|
||||
/done/ $ZERO_OID recurse valid
|
||||
/dthree/ $ZERO_OID recurse valid
|
||||
three
|
||||
/dtwo/ $ZERO_OID recurse valid
|
||||
two
|
||||
EOF
|
||||
|
||||
test_expect_success 'if -uall was configured, untracked cache is populated' '
|
||||
test-tool dump-untracked-cache >../actual &&
|
||||
test_cmp ../dump_uall.expect ../actual
|
||||
'
|
||||
|
||||
test_expect_success 'if -uall is configured, untracked cache is used by default' '
|
||||
test_config status.showuntrackedfiles all &&
|
||||
: >../trace.output &&
|
||||
GIT_TRACE2_PERF="$TRASH_DIRECTORY/trace.output" \
|
||||
git status --porcelain >../actual &&
|
||||
iuc status --porcelain >../status.iuc &&
|
||||
test_cmp ../status_uall.expect ../status.iuc &&
|
||||
test_cmp ../status_uall.expect ../actual &&
|
||||
get_relevant_traces ../trace.output ../trace.relevant &&
|
||||
cat >../trace.expect <<EOF &&
|
||||
....path:
|
||||
....node-creation:0
|
||||
....gitignore-invalidation:0
|
||||
....directory-invalidation:0
|
||||
....opendir:0
|
||||
EOF
|
||||
test_cmp ../trace.expect ../trace.relevant
|
||||
'
|
||||
|
||||
# Bypassing the untracked cache here is not desirable from an
|
||||
# end-user perspective, but is expected in the current design.
|
||||
# The untracked cache data stored for a -all run cannot be
|
||||
# correctly used in a -unormal run - it would yield incorrect
|
||||
# output.
|
||||
test_expect_success 'if -uall is configured, untracked cache is bypassed with -unormal' '
|
||||
test_config status.showuntrackedfiles all &&
|
||||
: >../trace.output &&
|
||||
GIT_TRACE2_PERF="$TRASH_DIRECTORY/trace.output" \
|
||||
git status -unormal --porcelain >../actual &&
|
||||
iuc status -unormal --porcelain >../status.iuc &&
|
||||
test_cmp ../status.expect ../status.iuc &&
|
||||
test_cmp ../status.expect ../actual &&
|
||||
get_relevant_traces ../trace.output ../trace.relevant &&
|
||||
cat >../trace.expect <<EOF &&
|
||||
....path:
|
||||
EOF
|
||||
test_cmp ../trace.expect ../trace.relevant
|
||||
'
|
||||
|
||||
test_expect_success 'repopulate untracked cache for -unormal' '
|
||||
git status --porcelain
|
||||
'
|
||||
|
||||
test_expect_success 'modify in root directory, one dir invalidation' '
|
||||
: >four &&
|
||||
test-tool chmtime =-240 four &&
|
||||
|
Loading…
Reference in New Issue
Block a user