Skip to content

Commit

Permalink
untracked-cache: support '--untracked-files=all' if configured
Browse files Browse the repository at this point in the history
Untracked cache was originally designed to only work with
'-untracked-files=normal', but this causes performance issues for UI
tooling that wants to see "all" on a frequent basis. On the other hand,
the conditions that prevented applicability to the "all" mode no
longer seem to apply.

The disqualification of untracked cache has a particularly significant
impact on Windows with the defaulted fscache, where the introduction
of fsmonitor can make the first and costly directory-iteration happen
in untracked file detection, single-threaded, rather than in
preload-index on multiple threads. Improving the performance of a
"normal" 'git status' run with fsmonitor can make
'git status --untracked-files=all' perform much worse.

To partially address this, align the supported directory flags for the
stored untracked cache data with the git config. If a user specifies
an '--untracked-files=' commandline parameter that does not align with
their 'status.showuntrackedfiles' config value, then the untracked
cache will be ignored - as it is for other unsupported situations like
when a pathspec is specified.

If the previously stored flags no longer match the current
configuration, but the currently-applicable flags do match the current
configuration, then discard the previously stored untracked cache
data.

For most users there will be no change in behavior. Users who need
'--untracked-files=all' to perform well will now have the option of
setting "status.showuntrackedfiles" to "all" for better / more
consistent performance.

Users who need '--untracked-files=all' to perform well for their
tooling AND prefer to avoid the verbosity of "all" when running
git status explicitly without options... are out of luck for now (no
change).

Users who have the "status.showuntrackedfiles" config set to "all"
and yet frequently explicitly call
'git status --untracked-files=normal' (and use the untracked cache)
are the only ones who will be disadvantaged by this change. Their
"--untracked-files=normal" calls will, after this change, no longer
use the untracked cache.

Signed-off-by: Tao Klerks <tao@klerks.biz>
  • Loading branch information
TaoK committed Feb 27, 2022
1 parent dab1b79 commit 49cf90b
Showing 1 changed file with 68 additions and 18 deletions.
86 changes: 68 additions & 18 deletions dir.c
Expand Up @@ -2746,13 +2746,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 configured_default_dir_flags(struct index_state *istate)
{
/* 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()
*/
char *status_untracked_files_config_value;
int config_outcome = repo_config_get_string(istate->repo,
"status.showuntrackedfiles",
&status_untracked_files_config_value);
if (!config_outcome && !strcmp(status_untracked_files_config_value, "all")) {
return 0;
} else {
/*
* 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, unsigned 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;
set_untracked_ident(uc);
istate->untracked = uc;
istate->cache_changed |= UNTRACKED_CHANGED;
Expand All @@ -2761,11 +2781,13 @@ 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,
configured_default_dir_flags(istate));
} else {
if (!ident_in_untracked(istate->untracked)) {
free_untracked_cache(istate->untracked);
new_untracked_cache(istate);
new_untracked_cache(istate,
configured_default_dir_flags(istate));
}
}
}
Expand All @@ -2781,10 +2803,12 @@ void remove_untracked_cache(struct index_state *istate)

static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *dir,
int base_len,
const struct pathspec *pathspec)
const struct pathspec *pathspec,
struct index_state *istate)
{
struct untracked_cache_dir *root;
static int untracked_cache_disabled = -1;
unsigned configured_dir_flags;

if (!dir->untracked)
return NULL;
Expand Down Expand Up @@ -2812,17 +2836,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;

/*
Expand All @@ -2845,6 +2861,40 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *d
return NULL;
}

/* We don't support using or preparing the untracked cache if
* the current effective flags don't match the configured
* flags.
*/
configured_dir_flags = configured_default_dir_flags(istate);
if (dir->flags != configured_dir_flags)
return NULL;

/* If the untracked structure we received does not have the same flags
* as configured, but the configured flags do match the effective flags,
* then we need to reset / create a new "untracked" structure to match
* the new config.
* Keeping the saved and used untracked cache in-line 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), where previously this option 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->flags != dir->untracked->dir_flags) {
free_untracked_cache(istate->untracked);
new_untracked_cache(istate, configured_dir_flags);
dir->untracked = istate->untracked;
}

if (!dir->untracked->root)
FLEX_ALLOC_STR(dir->untracked->root, name, "");

Expand Down Expand Up @@ -2916,7 +2966,7 @@ int read_directory(struct dir_struct *dir, struct index_state *istate,
return dir->nr;
}

untracked = validate_untracked_cache(dir, len, pathspec);
untracked = validate_untracked_cache(dir, len, pathspec, istate);
if (!untracked)
/*
* make sure untracked cache code path is disabled,
Expand Down

0 comments on commit 49cf90b

Please sign in to comment.