Skip to content

Commit bcc53ef

Browse files
gitstergitster
committed
Merge branch 'tk/untracked-cache-with-uall' into jch
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
2 parents faa21c1 + e6a6535 commit bcc53ef

File tree

2 files changed

+185
-16
lines changed

2 files changed

+185
-16
lines changed

‎dir.c‎

Lines changed: 72 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2747,13 +2747,33 @@ static void set_untracked_ident(struct untracked_cache *uc)
27472747
strbuf_addch(&uc->ident, 0);
27482748
}
27492749

2750-
static void new_untracked_cache(struct index_state *istate)
2750+
static unsigned new_untracked_cache_flags(struct index_state *istate)
2751+
{
2752+
struct repository *repo = istate->repo;
2753+
char *val;
2754+
2755+
/*
2756+
* This logic is coordinated with the setting of these flags in
2757+
* wt-status.c#wt_status_collect_untracked(), and the evaluation
2758+
* of the config setting in commit.c#git_status_config()
2759+
*/
2760+
if (!repo_config_get_string(repo, "status.showuntrackedfiles", &val) &&
2761+
!strcmp(val, "all"))
2762+
return 0;
2763+
2764+
/*
2765+
* The default, if "all" is not set, is "normal" - leading us here.
2766+
* If the value is "none" then it really doesn't matter.
2767+
*/
2768+
return DIR_SHOW_OTHER_DIRECTORIES | DIR_HIDE_EMPTY_DIRECTORIES;
2769+
}
2770+
2771+
static void new_untracked_cache(struct index_state *istate, int flags)
27512772
{
27522773
struct untracked_cache *uc = xcalloc(1, sizeof(*uc));
27532774
strbuf_init(&uc->ident, 100);
27542775
uc->exclude_per_dir = ".gitignore";
2755-
/* should be the same flags used by git-status */
2756-
uc->dir_flags = DIR_SHOW_OTHER_DIRECTORIES | DIR_HIDE_EMPTY_DIRECTORIES;
2776+
uc->dir_flags = flags >= 0 ? flags : new_untracked_cache_flags(istate);
27572777
set_untracked_ident(uc);
27582778
istate->untracked = uc;
27592779
istate->cache_changed |= UNTRACKED_CHANGED;
@@ -2762,11 +2782,11 @@ static void new_untracked_cache(struct index_state *istate)
27622782
void add_untracked_cache(struct index_state *istate)
27632783
{
27642784
if (!istate->untracked) {
2765-
new_untracked_cache(istate);
2785+
new_untracked_cache(istate, -1);
27662786
} else {
27672787
if (!ident_in_untracked(istate->untracked)) {
27682788
free_untracked_cache(istate->untracked);
2769-
new_untracked_cache(istate);
2789+
new_untracked_cache(istate, -1);
27702790
}
27712791
}
27722792
}
@@ -2814,17 +2834,9 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *d
28142834
if (base_len || (pathspec && pathspec->nr))
28152835
return NULL;
28162836

2817-
/* Different set of flags may produce different results */
2818-
if (dir->flags != dir->untracked->dir_flags ||
2819-
/*
2820-
* See treat_directory(), case index_nonexistent. Without
2821-
* this flag, we may need to also cache .git file content
2822-
* for the resolve_gitlink_ref() call, which we don't.
2823-
*/
2824-
!(dir->flags & DIR_SHOW_OTHER_DIRECTORIES) ||
2825-
/* We don't support collecting ignore files */
2826-
(dir->flags & (DIR_SHOW_IGNORED | DIR_SHOW_IGNORED_TOO |
2827-
DIR_COLLECT_IGNORED)))
2837+
/* We don't support collecting ignore files */
2838+
if (dir->flags & (DIR_SHOW_IGNORED | DIR_SHOW_IGNORED_TOO |
2839+
DIR_COLLECT_IGNORED))
28282840
return NULL;
28292841

28302842
/*
@@ -2847,6 +2859,50 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *d
28472859
return NULL;
28482860
}
28492861

2862+
/*
2863+
* If the untracked structure we received does not have the same flags
2864+
* as requested in this run, we're going to need to either discard the
2865+
* existing structure (and potentially later recreate), or bypass the
2866+
* untracked cache mechanism for this run.
2867+
*/
2868+
if (dir->flags != dir->untracked->dir_flags) {
2869+
/*
2870+
* If the untracked structure we received does not have the same flags
2871+
* as configured, then we need to reset / create a new "untracked"
2872+
* structure to match the new config.
2873+
*
2874+
* Keeping the saved and used untracked cache consistent with the
2875+
* configuration provides an opportunity for frequent users of
2876+
* "git status -uall" to leverage the untracked cache by aligning their
2877+
* configuration - setting "status.showuntrackedfiles" to "all" or
2878+
* "normal" as appropriate.
2879+
*
2880+
* Previously using -uall (or setting "status.showuntrackedfiles" to
2881+
* "all") was incompatible with untracked cache and *consistently*
2882+
* caused surprisingly bad performance (with fscache and fsmonitor
2883+
* enabled) on Windows.
2884+
*
2885+
* IMPROVEMENT OPPORTUNITY: If we reworked the untracked cache storage
2886+
* to not be as bound up with the desired output in a given run,
2887+
* and instead iterated through and stored enough information to
2888+
* correctly serve both "modes", then users could get peak performance
2889+
* with or without '-uall' regardless of their
2890+
* "status.showuntrackedfiles" config.
2891+
*/
2892+
if (dir->untracked->dir_flags != new_untracked_cache_flags(istate)) {
2893+
free_untracked_cache(istate->untracked);
2894+
new_untracked_cache(istate, dir->flags);
2895+
dir->untracked = istate->untracked;
2896+
}
2897+
else {
2898+
/*
2899+
* Current untracked cache data is consistent with config, but not
2900+
* usable in this request/run; just bypass untracked cache.
2901+
*/
2902+
return NULL;
2903+
}
2904+
}
2905+
28502906
if (!dir->untracked->root) {
28512907
/* Untracked cache existed but is not initialized; fix that */
28522908
FLEX_ALLOC_STR(dir->untracked->root, name, "");

‎t/t7063-status-untracked-cache.sh‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,119 @@ test_expect_success 'untracked cache after second status' '
190190
test_cmp ../dump.expect ../actual
191191
'
192192

193+
cat >../status_uall.expect <<EOF &&
194+
A done/one
195+
A one
196+
A two
197+
?? dthree/three
198+
?? dtwo/two
199+
?? three
200+
EOF
201+
202+
# Bypassing the untracked cache here is not desirable from an
203+
# end-user perspective, but is expected in the current design.
204+
# The untracked cache data stored for a -unormal run cannot be
205+
# correctly used in a -uall run - it would yield incorrect output.
206+
test_expect_success 'untracked cache is bypassed with -uall' '
207+
: >../trace.output &&
208+
GIT_TRACE2_PERF="$TRASH_DIRECTORY/trace.output" \
209+
git status -uall --porcelain >../actual &&
210+
iuc status -uall --porcelain >../status.iuc &&
211+
test_cmp ../status_uall.expect ../status.iuc &&
212+
test_cmp ../status_uall.expect ../actual &&
213+
get_relevant_traces ../trace.output ../trace.relevant &&
214+
cat >../trace.expect <<EOF &&
215+
....path:
216+
EOF
217+
test_cmp ../trace.expect ../trace.relevant
218+
'
219+
220+
test_expect_success 'untracked cache remains after bypass' '
221+
test-tool dump-untracked-cache >../actual &&
222+
test_cmp ../dump.expect ../actual
223+
'
224+
225+
test_expect_success 'if -uall is configured, untracked cache gets populated by default' '
226+
test_config status.showuntrackedfiles all &&
227+
: >../trace.output &&
228+
GIT_TRACE2_PERF="$TRASH_DIRECTORY/trace.output" \
229+
git status --porcelain >../actual &&
230+
iuc status --porcelain >../status.iuc &&
231+
test_cmp ../status_uall.expect ../status.iuc &&
232+
test_cmp ../status_uall.expect ../actual &&
233+
get_relevant_traces ../trace.output ../trace.relevant &&
234+
cat >../trace.expect <<EOF &&
235+
....path:
236+
....node-creation:3
237+
....gitignore-invalidation:1
238+
....directory-invalidation:0
239+
....opendir:4
240+
EOF
241+
test_cmp ../trace.expect ../trace.relevant
242+
'
243+
244+
cat >../dump_uall.expect <<EOF &&
245+
info/exclude $EMPTY_BLOB
246+
core.excludesfile $ZERO_OID
247+
exclude_per_dir .gitignore
248+
flags 00000000
249+
/ $ZERO_OID recurse valid
250+
three
251+
/done/ $ZERO_OID recurse valid
252+
/dthree/ $ZERO_OID recurse valid
253+
three
254+
/dtwo/ $ZERO_OID recurse valid
255+
two
256+
EOF
257+
258+
test_expect_success 'if -uall was configured, untracked cache is populated' '
259+
test-tool dump-untracked-cache >../actual &&
260+
test_cmp ../dump_uall.expect ../actual
261+
'
262+
263+
test_expect_success 'if -uall is configured, untracked cache is used by default' '
264+
test_config status.showuntrackedfiles all &&
265+
: >../trace.output &&
266+
GIT_TRACE2_PERF="$TRASH_DIRECTORY/trace.output" \
267+
git status --porcelain >../actual &&
268+
iuc status --porcelain >../status.iuc &&
269+
test_cmp ../status_uall.expect ../status.iuc &&
270+
test_cmp ../status_uall.expect ../actual &&
271+
get_relevant_traces ../trace.output ../trace.relevant &&
272+
cat >../trace.expect <<EOF &&
273+
....path:
274+
....node-creation:0
275+
....gitignore-invalidation:0
276+
....directory-invalidation:0
277+
....opendir:0
278+
EOF
279+
test_cmp ../trace.expect ../trace.relevant
280+
'
281+
282+
# Bypassing the untracked cache here is not desirable from an
283+
# end-user perspective, but is expected in the current design.
284+
# The untracked cache data stored for a -all run cannot be
285+
# correctly used in a -unormal run - it would yield incorrect
286+
# output.
287+
test_expect_success 'if -uall is configured, untracked cache is bypassed with -unormal' '
288+
test_config status.showuntrackedfiles all &&
289+
: >../trace.output &&
290+
GIT_TRACE2_PERF="$TRASH_DIRECTORY/trace.output" \
291+
git status -unormal --porcelain >../actual &&
292+
iuc status -unormal --porcelain >../status.iuc &&
293+
test_cmp ../status.expect ../status.iuc &&
294+
test_cmp ../status.expect ../actual &&
295+
get_relevant_traces ../trace.output ../trace.relevant &&
296+
cat >../trace.expect <<EOF &&
297+
....path:
298+
EOF
299+
test_cmp ../trace.expect ../trace.relevant
300+
'
301+
302+
test_expect_success 'repopulate untracked cache for -unormal' '
303+
git status --porcelain
304+
'
305+
193306
test_expect_success 'modify in root directory, one dir invalidation' '
194307
: >four &&
195308
test-tool chmtime =-240 four &&

0 commit comments

Comments
 (0)