@matheusbsbs / matheus-bagliano / commits / e6a653554b

untracked-cache: support '--untracked-files=all' if configured

Untracked cache was originally designed to only work with "--untracked-files=normal", and is bypassed when "--untracked-files=all" is requested, but this causes performance issues for UI tooling that wants to see "all" on a frequent basis. On the other hand, the conditions that altogether prevented applicability to the "all" mode no longer seem to apply, after several major refactors in recent years; this possibility was discussed in 81153d02-8e7a-be59-e709-e90cd5906f3a@jeffhostetler.com and CABPp-BFiwzzUgiTj_zu+vF5x20L0=1cf25cHwk7KZQj2YkVzXw@mail.gmail.com, and somewhat confirmed experimentally by several users using a version of this patch to use untracked cache with -uall for about a year. When 'git status' runs without using the untracked cache, on a large repo, on windows, with fsmonitor, it can run very slowly. This can make GUIs that need to use "-uall" (and therefore currently bypass untracked cache) unusable when fsmonitor is enabled, on such large repos. 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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Tao Klerks committed Mar 31, 2022 at 16:02 UTC e6a653554bb49c26d105f3b478cbdbb1c0648f65

2 files changed +153 -16

dir.c

+72 -16

@@ -2747,13 +2747,33 @@ static void set_untracked_ident(struct untracked_cache *uc)

2747 strbuf_addch(&uc->ident, 0);

2748 }

2749

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)

2772 {

2773 struct untracked_cache *uc = xcalloc(1, sizeof(*uc));

2774 strbuf_init(&uc->ident, 100);

2775 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);

2777 set_untracked_ident(uc);

2778 istate->untracked = uc;

2779 istate->cache_changed |= UNTRACKED_CHANGED;

@@ -2762,11 +2782,11 @@ static void new_untracked_cache(struct index_state *istate)

2782 void add_untracked_cache(struct index_state *istate)

2783 {

2784 if (!istate->untracked) {

2765 - new_untracked_cache(istate);

2785 + new_untracked_cache(istate, -1);

2786 } else {

2787 if (!ident_in_untracked(istate->untracked)) {

2788 free_untracked_cache(istate->untracked);

2769 - new_untracked_cache(istate);

2789 + new_untracked_cache(istate, -1);

2790 }

2791 }

2792 }

@@ -2814,17 +2834,9 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *d

2834 if (base_len || (pathspec && pathspec->nr))

2835 return NULL;

2836

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))

2840 return NULL;

2841

2842 /*

@@ -2847,6 +2859,50 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *d

2859 return NULL;

2860 }

2861

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 +

2906 if (!dir->untracked->root) {

2907 /* Untracked cache existed but is not initialized; fix that */

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

t/t7063-status-untracked-cache.sh

+81

@@ -222,6 +222,87 @@ test_expect_success 'untracked cache remains after bypass' '

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 +

306 test_expect_success 'modify in root directory, one dir invalidation' '

307 : >four &&

308 test-tool chmtime =-240 four &&