fsmonitor: long status advice adapted to the fsmonitor use case

From: Rudy Rigot <[email protected]>

Currently, if git-status takes more than 2 seconds for enumerating untracked files, a piece of advice is given to the user to consider ignoring untracked files. This is somewhat at odds with the UX upsides from having fsmonitor enabled, since fsmonitor will be here to take care of mitigating the performance downsides from those untracked files.

I considered just suppressing that piece of advice entirely for repositories with fsmonitor disabled, but I decided to replace it with another piece of advice instead, letting the user know that this run may have been slow, but the next ones should be faster. Of course, please let me know if the phrasing can be improved. To keep consistent with other pieces of advice, this new one can be hidden with a new advice.statusFsmonitor config.

If the repository does not have fsmonitor enabled, or if the new piece of advice is hidden by config, the behavior falls back to today's behavior: show the message advising to ignore untracked files, as long as it wasn't disabled with the existing advice.statusUoption config.

Test-wise, I tried to figure out ways to mock the behavior of a slow git-status, but I couldn't figure it out, so I could use some advice. I tracked down Commit 6a38ef2ced (status: advise to consider use of -u when read_directory takes too long, 2013-03-13), and it also didn't have tests, so I'm questioning whether it can in fact be reasonably done. Thanks in advance for any guidance.

---
fsmonitor: long status advice adapted to the fsmonitor use case

Currently, if git-status takes more than 2 seconds for enumerating untracked files, a piece of advice is given to the user to consider ignoring untracked files. This is somewhat at odds with the UX upsides from having fsmonitor enabled, since fsmonitor will be here to take care of mitigating the performance downsides from those untracked files.

I considered just suppressing that piece of advice entirely for repositories with fsmonitor disabled, but I decided to replace it with another piece of advice instead, letting the user know that this run may have been slow, but the next ones should be faster. Of course, please let me know if the phrasing can be improved. To keep consistent with other pieces of advice, this new one can be hidden with a new advice.statusFsmonitor config.

If the repository does not have fsmonitor enabled, or if the new piece of advice is hidden by config, the behavior falls back to today's
behavior: show the message advising to ignore untracked files, as long as it wasn't disabled with the existing advice.statusUoption config.

Test-wise, I tried to figure out ways to mock the behavior of a slow git-status, but I couldn't figure it out, so I could use some advice. I tracked down Commit 6a38ef2 (status: advise to consider use of -u when read_directory takes too long, 2013-03-13), and it also didn't have tests, so I'm questioning whether it can in fact be reasonably done. Thanks in advance for any guidance.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1384%2Frudyrigot%2Fadvice_statusFsmonitor-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1384/rudyrigot/advice_statusFsmonitor-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1384

 Documentation/config/advice.txt |  3 +++
 advice.c                        |  1 +
 advice.h                        |  1 +
 wt-status.c                     | 23 ++++++++++++++++-------
 4 files changed, 21 insertions(+), 7 deletions(-)

index a00d0100a82..e8ebcf1b023 100644
@@ -57,6 +57,9 @@ advice.*::
 		and that calculation takes longer than expected. Will not
 		appear if `status.aheadBehind` is false or the option
 		`--no-ahead-behind` is given.
+	statusFsmonitor::
+		Shown when the linkgit:git-status[1] command takes more than
+		2 seconds to enumerate untracked files, and fsmonitor is enabled.
 	statusHints::
 		Show directions on how to proceed from the current
 		state in the output of linkgit:git-status[1], in

index fd189689437..448b11ef273 100644
@@ -69,6 +69,7 @@ static struct {
 	[ADVICE_SET_UPSTREAM_FAILURE]			= { "setUpstreamFailure", 1 },
 	[ADVICE_SKIPPED_CHERRY_PICKS]			= { "skippedCherryPicks", 1 },
 	[ADVICE_STATUS_AHEAD_BEHIND_WARNING]		= { "statusAheadBehindWarning", 1 },
+	[ADVICE_STATUS_FSMONITOR]			= { "statusFsmonitor", 1 },
 	[ADVICE_STATUS_HINTS]				= { "statusHints", 1 },
 	[ADVICE_STATUS_U_OPTION]			= { "statusUoption", 1 },
 	[ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE] = { "submoduleAlternateErrorStrategyDie", 1 },

index 07e0f76833e..a458619a160 100644
@@ -43,6 +43,7 @@ struct string_list;
 	ADVICE_SEQUENCER_IN_USE,
 	ADVICE_SET_UPSTREAM_FAILURE,
 	ADVICE_STATUS_AHEAD_BEHIND_WARNING,
+	ADVICE_STATUS_FSMONITOR,
 	ADVICE_STATUS_HINTS,
 	ADVICE_STATUS_U_OPTION,
 	ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE,

index 5813174896c..d6c7f0fa21a 100644
@@ -18,6 +18,7 @@
 #include "worktree.h"
 #include "lockfile.h"
 #include "sequencer.h"
+#include "fsmonitor-settings.h"
 
 #define AB_DELAY_WARNING_IN_MS (2 * 1000)
 

@@ -1814,6 +1815,7 @@ static void wt_longstatus_print(struct wt_status *s)
 {
 	const char *branch_color = color(WT_STATUS_ONBRANCH, s);
 	const char *branch_status_color = color(WT_STATUS_HEADER, s);
+	enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(s->repo);
 
 	if (s->branch) {
 		const char *on_what = _("On branch ");

@@ -1870,13 +1872,20 @@ static void wt_longstatus_print(struct wt_status *s)
 		wt_longstatus_print_other(s, &s->untracked, _("Untracked files"), "add");
 		if (s->show_ignored_mode)
 			wt_longstatus_print_other(s, &s->ignored, _("Ignored files"), "add -f");
-		if (advice_enabled(ADVICE_STATUS_U_OPTION) && 2000 < s->untracked_in_ms) {
-			status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
-			status_printf_ln(s, GIT_COLOR_NORMAL,
-					 _("It took %.2f seconds to enumerate untracked files. 'status -uno'\n"
-					   "may speed it up, but you have to be careful not to forget to add\n"
-					   "new files yourself (see 'git help status')."),
-					 s->untracked_in_ms / 1000.0);
+		if (2000 < s->untracked_in_ms) {
+			if (advice_enabled(ADVICE_STATUS_FSMONITOR) && fsm_mode > FSMONITOR_MODE_DISABLED) {
+				status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
+				status_printf_ln(s, GIT_COLOR_NORMAL,
+						_("It took a while to check your git status this time, but the results\n"
+						"were cached, and your next runs should be faster."));
+			} else if (advice_enabled(ADVICE_STATUS_U_OPTION)) {
+				status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
+				status_printf_ln(s, GIT_COLOR_NORMAL,
+						_("It took %.2f seconds to enumerate untracked files. 'status -uno'\n"
+						"may speed it up, but you have to be careful not to forget to add\n"
+						"new files yourself (see 'git help status')."),
+						s->untracked_in_ms / 1000.0);
+			}
 		}
 	} else if (s->committable)
 		status_printf_ln(s, GIT_COLOR_NORMAL, _("Untracked files not listed%s"),

base-commit: d420dda0576340909c3faff364cfbd1485f70376

gitgitgadget

repositories with fsmonitor disabled

Sorry, I just noticed that I misspoke here. I meant repositories with fsmonitor *enabled*, of course.

Yes, the original advice needs some updating. Thanks!

We should be careful here, FSMonitor only helps with untracked files if the untracked cache (UC) is also turned on. They do work well together and they greatly speed up things, but if either is turned off, git status will still need to scan. (Small nerd
detail: the UC by itself does speed things up -- it only needs to lstat known directories most of the time, rather than actually readdir them. When both are on, we don't even need to do that.)

The original message suggested turning off the untracked file (UF) scan completely. Perhaps, we could have advice say to turn off UF scan -or- turn on FSM & UC ?

We also need to be careful here. With FSMonitor the "first one is slow, the second one is fast" happens because git status is secretly updating the index (despite looking like a read-only command). That causes the index to have an updated FSM token (so that subsequent FSM responses are relative to a more recent checkpoint). See [1]. So it isn't always correct that the second status will be faster. It usually is, but it just depends on the threshold -- and more importantly, that the UC is turned on. (So I guess what I'm saying is that again, we should advise to turn UF or turn on both FSM & UC.)

[1] 26b9f34ab3 (fsmonitor: force update index after large responses, 2022-03-25)

[...]

WRT testing, you might do something like:

	#define UF_DELAY_WARNING_IN_MS (2 * 1000)

	static inline int uf_was_slow(uint32_t untracked_in_ms)
	{
	#ifdef GIT_TEST_UF_DELAY_WARNING // or maybe use getenv() here
		untracked_in_ms += UF_DELAY_WARNING_IN_MS + 1;
	#endif

		return UF_DELAY_WARNING_IN_MS < untracked_in_ms;
	}

And then you can set the GIT_TEST_ symbol (or env var) during the test scripts and confirm that we get the messages that you expect.

Hope this helps,
Jeff

files if the untracked cache (UC) is also turned on. They do work well together and they greatly speed up things, but if either is turned off, git status will still need to scan.

Oh, thanks, I didn't realize! With that, I agree that the messaging I'm proposing is not technically correct, and needs to be fixed.

I agree with your advice about the message when FSMonitor and untracked cache are both turned off. I'm also trying to think about what advice to give when they are turned on, and git status was slow because the update-index was building the cache on that call.

Importantly, I'm trying to think of ways to keep the messaging accessible even when the user is not familiar with those concepts. I'm thinking a user may not even know what is currently enabled or not in their environment, so there's probably value in detecting their situation, and best adapting the messaging to it.

For context, in our case, we set core.fsmonitor and core.untrackedCache as part of our dev environment setup script, because we don't expect our least advanced developers to ramp up on what they are. And yet, it is useful to all of them to have them enabled, our git status is about 30 seconds long without FSMonitor and UC.

As a result, we have been receiving negative feedback that git status is slow, but when we inform the user that it is cached, they run it again and confirm that it is fine like this. The problem being about educating users and not a technical issue, of course we're adding the info to our setup doc, but I figured other large repos may hit this usability issue, so here I am.

What do you think of those phrasings?

- If neither FSMonitor nor the untracked cache are turned on, changing the current advice to: "It took %.2f seconds to enumerate untracked files. You may want to skip that part with 'status -uno' but you have to be careful not to forget to add new files yourself (see 'git help status'). Otherwise, you can enable the core.untrackedCache config to have it be cached, and potentially the core.fsmonitor config to further improve the cache's performance."

- If only the untracked cache is turned on, since you said it could already improve some: "It took %.2f seconds to enumerate untracked files. Your untracked cache is enabled, but you may want to enable the core.fsmonitor config to further improve the cache's performance. Otherwise, you may want to skip that part with 'status -uno' but you have to be careful not to forget to add new files yourself (see 'git help status')."

- If they're both turned on: "It took %.2f seconds to enumerate untracked files. Your untracked cache is enabled and fsmonitor is on,
so your next calls may be faster. Otherwise, you may want to skip that part with 'status -uno' but you have to be careful not to forget to add new files yourself (see 'git help status')."

I intentionally phrased it all in a way that manages expectations, "may be faster", since you said the state of the FSM token could be impacted by a variety of things, so we can't exactly commit to anything for sure about the next call.

Let me know what you think, I definitely want to do this right.

Oh, I didn't realize I could do this! Alright, I need time to look into it. With that, it means I can test the existing advice message use case, which I don't believe is currently tested, so I'm double-glad!

Thanks a lot for all the insight.

Agreed. We sometimes forget that not everyone is an expert in the subtleties and terms of Git. Even the original message "...enumerate untracked files..." can be a little obscure to the casual user.

All three of these are a bit wordy and/or awkward -- but I'm not sure how to say it either, so don't feel bad. I think it is just the quality of the corner that we've been backed into -- there are just too many knobs and not enough space to do them justice. And I've always found the "turn this on, but be careful..." advice/warning a bit odd.

As an alternative, and definitely just thinking out loud here.

Would it be better to add a "Untracked Files and Status Speed" section to the bottom of https://git-scm.com/docs/git-status
(aka Documentation/git-status.txt) near
https://git-scm.com/docs/git-status#_background_refresh

Describe the various combination of config options in more detail and discuss the pros/cons of each, what the defaults are, and how to set them, how to tell what they are currently set to, and so on.

Then have the advice message reference that new section.

We might also reword the large paragraph ("When -u option is not...") in the --untracked-files argument to reference the new section. It is a little stale and just as awkward.

Just a thought...

Jeff

Oooh, I definitely like where your mind is with this. I think it makes a lot of sense, there used to be one way to act upon a slow status, now there are a few, so I can see how any in-depth explanation here would add to the confusion for a user in a terminal who is just trying to get things done. And I see how the current messaging already kinda infringes on that.

Alright, I can write the first draft of the documentation changes you were mentioning. Heads up though: I'm going to need your tight review of it, because I'm not as completely comfortable with what each option does as I wish I was, so I worry I may write something accidentally inaccurate. I'll take the time to read up on it though, and then I'll try my hand at it and update it here, and let's take it from there if that sounds good.

With that, I'm thinking the "slow status" advice could be turned into something as simple as:

(To be clear, I'm very un-opinionated about phrasing.)

There's one thing I'm still worried about though, which is what you mentioned earlier, and which brought me here: the fact that git-status feels like a read-only command, but secretly isn't. I'm thinking the confusing use case is when the repository was in fact set in a way that things are cached, and yet git-status is still slow because it was generating that cache, and the user doesn't have a way to know that.

Adding to the murkiness, it might not have been the reason, so I understand we can't say things in such a confident manner as "it will be faster now", because of course it depends.

So I'm thinking, after the message above when git-status was slow, in the specific case where the untracked cache is on (whether FSMonitor is on or not, since that sounds more like under-the-hood detail), we could display something like the additional line here:

If the untracked cache is on, I'm assuming that would be always accurate information, is that correct?

If you're concerned that users may not understand what it means for them, we could also make it more obvious without over-committing about it:

What do you think?

To summarize, next steps for me:
- Make the first draft for the doc updates.
- Change the advice messaging based on our discussion above and what you think of it.
- I still need to look into your test-related advice from last time, I haven't yet. I really would like to give tests to all this.

Thanks a lot for all this, it helps tremendously!

yeah, i'm happy to help. just let me know when you have a first draft.

Jeff

From: Rudy Rigot <[email protected]>

Currently, if git-status takes more than 2 seconds for enumerating untracked files, a piece of advice is given to the user to consider ignoring untracked files. But Git now offers more possibilities to resolve that situation (untracked cache, fsmonitor) with different downsides.

This change is about refreshing that advice message. A new section in the documentation is introduced to present the possibilities, and the advice message links to it. I'm also introducing tests for this advice message, which was untested so far.

One of the downsides of untracked cache / fsmonitor, is that the first call may be long in order to generate the cache, but the user may not know what their current configuration is. When collecting feedback from users of our very large repo, that's the most common point of confusion that keeps coming
back: people complain about git status being slow, but are satisfied when we inform them that it's being cached and they should run it again to check. As a result, the advice message tries to keep them informed of their current configuration.

---
status: long status advice adapted to recent capabilities

Here is version 2 for this patch to change the long status advice to adapt to recent capabilities (untracked cache, FSMonitor).

Clerical note: I /preview'd this with GitGitGadget, and this seems to be expressed as a patch on top of my first patch ("Range-diff vs v1") first, before showing the actual patch as refreshed. I'm assuming that is what is expected, please let me know if I'm doing this wrong.

Changes since v1:

* Introduced a new section in git status's docs to explain the various options, that the new advice can link to. I realized there's already solid details about untracked cache and fsmonitor in the update-index's docs, so I kept the new section very high-level and strategic, linking to update-index's docs for more details. I don't think anything's inaccurate, but I really could use some eyes on this anyway to be sure.
* Changed the advice message depending on context (see test files to see every possible phrasing the end user can meet). I am very open to feedback, of course. One of my priorities was to keep the already-optimized user in the loop about what optimizations they may already have, in case they're only seeing this message because it's the git status run that did the first caching, since that's what's been confusing our users when using fsmonitor.
* Introduced tests for all this. I chose to simulate the slowness thanks to a an environment variable rather than a symbol, just because it was also helpful to set it from the outside at dev time, but I'm not strongly opinionated about whether it should be one or the other.
* Removed the advice.statusFsmonitor config I had introduced in v1, since it's all one short consolidated advice message now.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1384%2Frudyrigot%2Fadvice_statusFsmonitor-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1384/rudyrigot/advice_statusFsmonitor-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1384

Range-diff vs v1:

1: cc372e7c9d0 ! 1: 9ef7f1834b7 fsmonitor: long status advice adapted to the fsmonitor use case
@@ Metadata
Author: Rudy Rigot <[email protected]>

## Commit message ##
- fsmonitor: long status advice adapted to the fsmonitor use case
+ status: long status advice adapted to recent capabilities

- Currently, if git-status takes more than 2 seconds for enumerating
- untracked files, a piece of advice is given to the user to consider
- ignoring untracked files. This is somewhat at odds with the UX
- upsides from having fsmonitor enabled, since fsmonitor will be
- here to take care of mitigating the performance downsides from
- those untracked files.
+ Currently, if git-status takes more than 2 seconds for enumerating untracked
+ files, a piece of advice is given to the user to consider ignoring untracked
+ files. But Git now offers more possibilities to resolve that situation
+ (untracked cache, fsmonitor) with different downsides.

- I considered just suppressing that piece of advice entirely for
- repositories with fsmonitor disabled, but I decided to replace
- it with another piece of advice instead, letting the user know
- that this run may have been slow, but the next ones should be faster.
- Of course, please let me know if the phrasing can be improved. To
- keep consistent with other pieces of advice, this new one can be
- hidden with a new advice.statusFsmonitor config.
+ This change is about refreshing that advice message. A new section in the
+ documentation is introduced to present the possibilities, and the advice
+ message links to it. I'm also introducing tests for this advice message,
+ which was untested so far.

- If the repository does not have fsmonitor enabled, or if the new
- piece of advice is hidden by config, the behavior falls back to
- today's behavior: show the message advising to ignore untracked
- files, as long as it wasn't disabled with the existing advice.statusUoption
- config.
-
- Test-wise, I tried to figure out ways to mock the behavior of a
- slow git-status, but I couldn't figure it out, so I could use some
- advice. I tracked down Commit 6a38ef2ced (status: advise to consider
- use of -u when read_directory takes too long, 2013-03-13), and it
- also didn't have tests, so I'm questioning whether it can in fact
- be reasonably done. Thanks in advance for any guidance.
+ One of the downsides of untracked cache / fsmonitor, is that the first call
+ may be long in order to generate the cache, but the user may not know what
+ their current configuration is. When collecting feedback from users of our
+ very large repo, that's the most common point of confusion that keeps coming
+ back: people complain about git status being slow, but are satisfied when
+ we inform them that it's being cached and they should run it again to check.
+ As a result, the advice message tries to keep them informed of their current
+ configuration.

- ## Documentation/config/advice.txt ##
-@@ Documentation/config/advice.txt: advice.*::
- and that calculation takes longer than expected. Will not
- appear if status.aheadBehind is false or the option
- --no-ahead-behind is given.
-+ statusFsmonitor::
-+ Shown when the linkgit:git-status[1] command takes more than
-+ 2 seconds to enumerate untracked files, and fsmonitor is enabled.
- statusHints::
- Show directions on how to proceed from the current
- state in the output of linkgit:git-status[1], in
+ ## Documentation/git-status.txt ##
+@@ Documentation/git-status.txt: during the write may conflict with other simultaneous processes, causing
+ them to fail. Scripts running status in the background should consider
+ using git --no-optional-locks status (see linkgit:git[1] for details).
+
++UNTRACKED FILES AND STATUS SPEED
++--------------------------------
++
++If your untracked files take an unusual amount of time to enumerate, your
++repository certainly has a lot of them, and an advice message will display
++about it. Here are some configurations to consider in order to improve the ++situation:
++
++* Setting the core.untrackedCache configuration as true will allow for
++git status to keep track of the mtime of folders, in order to cache past
++status results and be sure to only browse folders that changed on subsequent
++runs, for filesystems that can support it (see linkgit:git-update-index[1]
++for details).
++* Used in conjonction with core.untrackedCache, setting the core.fsmonitor ++configuration as true will allow for git status to keep track of what
++files recently changed, in order to cache past status results and be sure
++to only focus on those files on subsequent runs (see linkgit:git-update-index[1]
++for details).
++* If none of the above options are satisfactory, setting the ++status.showUntrackedFiles configuration as no will cause git status
++to not attempt to list untracked files anymore, in which case you have to be
++careful not to forget to add new files yourself.
++
++If none of the above solutions are satisfactory, and you are bothered with
++the advice message, you can disable it by setting the advice.statusUoption ++configuration to false.
++
+ SEE ALSO
+ --------
+ linkgit:gitignore[5]
+
+ ## t/t7065-wtstatus-slow.sh (new) ##
+@@
++#!/bin/sh
++
++test_description='test status when slow untracked files'
++
++. ./test-lib.sh
++
++DATA="$TEST_DIRECTORY/t7065"
++
++GIT_TEST_UF_DELAY_WARNING=1
++export GIT_TEST_UF_DELAY_WARNING
++
++test_expect_success setup '
++ git checkout -b test
++'
++
++test_expect_success 'when core.untrackedCache and fsmonitor are unset' '
++ test_must_fail git config --get core.untrackedCache &&
++ test_must_fail git config --get core.fsmonitor &&
++ git status | sed "s/[0-9]\.[0-9][0-9]/X/g" >../actual &&
++ test_cmp "$DATA/no_untrackedcache_no_fsmonitor" ../actual &&
++ rm -fr ../actual
++'
++
++test_expect_success 'when core.untrackedCache true, but not fsmonitor' '
++ git config core.untrackedCache true &&
++ test_must_fail git config --get core.fsmonitor &&
++ git status | sed "s/[0-9]\.[0-9][0-9]/X/g" >../actual &&
++ test_cmp "$DATA/with_untrackedcache_no_fsmonitor" ../actual &&
++ rm -fr ../actual
++'
++
++test_expect_success 'when core.untrackedCache true, and fsmonitor' '
++ git config core.untrackedCache true &&
++ git config core.fsmonitor true &&
++ git status | sed "s/[0-9]\.[0-9][0-9]/X/g" >../actual &&
++ test_cmp "$DATA/with_untrackedcache_with_fsmonitor" ../actual &&
++ rm -fr ../actual
++'
++
++test_done
+ \ No newline at end of file

- ## advice.c ##
-@@ advice.c: static struct {
- [ADVICE_SET_UPSTREAM_FAILURE] = { "setUpstreamFailure", 1 },
- [ADVICE_SKIPPED_CHERRY_PICKS] = { "skippedCherryPicks", 1 },
- [ADVICE_STATUS_AHEAD_BEHIND_WARNING] = { "statusAheadBehindWarning", 1 },
-+ [ADVICE_STATUS_FSMONITOR] = { "statusFsmonitor", 1 },
- [ADVICE_STATUS_HINTS] = { "statusHints", 1 },
- [ADVICE_STATUS_U_OPTION] = { "statusUoption", 1 },
- [ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE] = { "submoduleAlternateErrorStrategyDie", 1 },
+ ## t/t7065/no_untrackedcache_no_fsmonitor (new) ##
+@@
++On branch test
++
++No commits yet
++
++
++It took X seconds to enumerate untracked files.
++See https://git-scm.com/docs/git-status#_untracked_files_and_status_speed
++for configuration options that may improve that time.
++
++nothing to commit (create/copy files and use "git add" to track)

- ## advice.h ##
-@@ advice.h: struct string_list;
- ADVICE_SEQUENCER_IN_USE,
- ADVICE_SET_UPSTREAM_FAILURE,
- ADVICE_STATUS_AHEAD_BEHIND_WARNING,
-+ ADVICE_STATUS_FSMONITOR,
- ADVICE_STATUS_HINTS,
- ADVICE_STATUS_U_OPTION,
- ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE,
+ ## t/t7065/with_untrackedcache_no_fsmonitor (new) ##
+@@
++On branch test
++
++No commits yet
++
++
++It took X seconds to enumerate untracked files,
++but this is currently being cached, with fsmonitor OFF. ++See https://git-scm.com/docs/git-status#_untracked_files_and_status_speed
++for configuration options that may improve that time.
++
++nothing to commit (create/copy files and use "git add" to track) +
+ ## t/t7065/with_untrackedcache_with_fsmonitor (new) ##
+@@
++On branch test
++
++No commits yet
++
++
++It took X seconds to enumerate untracked files,
++but this is currently being cached, with fsmonitor ON.
++See https://git-scm.com/docs/git-status#_untracked_files_and_status_speed
++for configuration options that may improve that time.
++
++nothing to commit (create/copy files and use "git add" to track)

## wt-status.c ##
@@
@@ wt-status.c
+#include "fsmonitor-settings.h"

#define AB_DELAY_WARNING_IN_MS (2 * 1000)
++#define UF_DELAY_WARNING_IN_MS (2 * 1000)
+
+ static const char cut_line[] =
+ "------------------------ >8 ------------------------\n";
+@@ wt-status.c: static void wt_longstatus_print_tracking(struct wt_status *s)
+ strbuf_release(&sb);
+ }

++static inline int uf_was_slow(uint32_t untracked_in_ms) ++{
++ const char *x;
++ x = getenv("GIT_TEST_UF_DELAY_WARNING");
++ if (x) {
++ untracked_in_ms += UF_DELAY_WARNING_IN_MS + 1;
++ }
++
++ return UF_DELAY_WARNING_IN_MS < untracked_in_ms;
++}
++
+ static void show_merge_in_progress(struct wt_status *s,
+ const char *color)
+ {
@@ wt-status.c: static void wt_longstatus_print(struct wt_status *s)
{
const char *branch_color = color(WT_STATUS_ONBRANCH, s);
@@ wt-status.c: static void wt_longstatus_print(struct wt_status *s)
- "may speed it up, but you have to be careful not to forget to add\n"
- "new files yourself (see 'git help status')."),
- s->untracked_in_ms / 1000.0);
-+ if (2000 < s->untracked_in_ms) {
-+ if (advice_enabled(ADVICE_STATUS_FSMONITOR) && fsm_mode > FSMONITOR_MODE_DISABLED) {
++ if (uf_was_slow(s->untracked_in_ms)) {
++ if (advice_enabled(ADVICE_STATUS_U_OPTION)) {
+ status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
++ if (s->repo->settings.core_untracked_cache == UNTRACKED_CACHE_WRITE) {
++ status_printf_ln(s, GIT_COLOR_NORMAL,
++ _("It took %.2f seconds to enumerate untracked files,\n"
++ "but this is currently being cached, with fsmonitor %s."),
++ s->untracked_in_ms / 1000.0,
++ (fsm_mode > FSMONITOR_MODE_DISABLED) ? "ON" : "OFF");
++ } else {
++ status_printf_ln(s, GIT_COLOR_NORMAL,
++ _("It took %.2f seconds to enumerate untracked files."),
++ s->untracked_in_ms / 1000.0);
++ }
+ status_printf_ln(s, GIT_COLOR_NORMAL,
-+ _("It took a while to check your git status this time, but the results\n"
-+ "were cached, and your next runs should be faster."));
-+ } else if (advice_enabled(ADVICE_STATUS_U_OPTION)) {
++ _("See https://git-scm.com/docs/git-status#_untracked_files_and_status_speed\n"
++ "for configuration options that may improve that time."));
+ status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
-+ status_printf_ln(s, GIT_COLOR_NORMAL,
-+ _("It took %.2f seconds to enumerate untracked files. 'status -uno'\n"
-+ "may speed it up, but you have to be careful not to forget to add\n"
-+ "new files yourself (see 'git help status')."),
-+ s->untracked_in_ms / 1000.0);
+ }
}
} else if (s->committable)

 Documentation/git-status.txt               | 27 +++++++++++++++
 t/t7065-wtstatus-slow.sh                   | 40 ++++++++++++++++++++++
 t/t7065/no_untrackedcache_no_fsmonitor     | 10 ++++++
 t/t7065/with_untrackedcache_no_fsmonitor   | 11 ++++++
 t/t7065/with_untrackedcache_with_fsmonitor | 11 ++++++
 wt-status.c                                | 40 ++++++++++++++++++----
 6 files changed, 132 insertions(+), 7 deletions(-)
 create mode 100755 t/t7065-wtstatus-slow.sh
 create mode 100644 t/t7065/no_untrackedcache_no_fsmonitor
 create mode 100644 t/t7065/with_untrackedcache_no_fsmonitor
 create mode 100644 t/t7065/with_untrackedcache_with_fsmonitor

index 54a4b29b473..3d92e5fd018 100644
@@ -457,6 +457,33 @@ during the write may conflict with other simultaneous processes, causing
 them to fail. Scripts running `status` in the background should consider
 using `git --no-optional-locks status` (see linkgit:git[1] for details).
 
+UNTRACKED FILES AND STATUS SPEED
+--------------------------------
+
+If your untracked files take an unusual amount of time to enumerate, your
+repository certainly has a lot of them, and an advice message will display
+about it. Here are some configurations to consider in order to improve the
+situation:
+
+* Setting the `core.untrackedCache` configuration as `true` will allow for
+`git status` to keep track of the mtime of folders, in order to cache past
+`status` results and be sure to only browse folders that changed on subsequent
+runs, for filesystems that can support it (see linkgit:git-update-index[1]
+for details).
+* Used in conjonction with `core.untrackedCache`, setting the `core.fsmonitor`
+configuration as `true` will allow for `git status` to keep track of what
+files recently changed, in order to cache past `status` results and be sure
+to only focus on those files on subsequent runs (see linkgit:git-update-index[1]
+for details).
+* If none of the above options are satisfactory, setting the
+`status.showUntrackedFiles` configuration as `no` will cause `git status`
+to not attempt to list untracked files anymore, in which case you have to be
+careful not to forget to add new files yourself.
+
+If none of the above solutions are satisfactory, and you are bothered with
+the advice message, you can disable it by setting the `advice.statusUoption`
+configuration to `false`.
+
 SEE ALSO
 --------
 linkgit:gitignore[5]

new file mode 100755
index 00000000000..92c053eaa64
@@ -0,0 +1,40 @@
+#!/bin/sh
+
+test_description='test status when slow untracked files'
+
+. ./test-lib.sh
+
+DATA="$TEST_DIRECTORY/t7065"
+
+GIT_TEST_UF_DELAY_WARNING=1
+export GIT_TEST_UF_DELAY_WARNING
+
+test_expect_success setup '
+	git checkout -b test
+'
+
+test_expect_success 'when core.untrackedCache and fsmonitor are unset' '
+	test_must_fail git config --get core.untrackedCache &&
+	test_must_fail git config --get core.fsmonitor &&
+    git status | sed "s/[0-9]\.[0-9][0-9]/X/g" >../actual &&
+    test_cmp "$DATA/no_untrackedcache_no_fsmonitor" ../actual &&
+    rm -fr ../actual
+'
+
+test_expect_success 'when core.untrackedCache true, but not fsmonitor' '
+    git config core.untrackedCache true &&
+	test_must_fail git config --get core.fsmonitor &&
+    git status | sed "s/[0-9]\.[0-9][0-9]/X/g" >../actual &&
+    test_cmp "$DATA/with_untrackedcache_no_fsmonitor" ../actual &&
+    rm -fr ../actual
+'
+
+test_expect_success 'when core.untrackedCache true, and fsmonitor' '
+    git config core.untrackedCache true &&
+	git config core.fsmonitor true &&
+    git status | sed "s/[0-9]\.[0-9][0-9]/X/g" >../actual &&
+    test_cmp "$DATA/with_untrackedcache_with_fsmonitor" ../actual &&
+    rm -fr ../actual
+'
+
+test_done
\ No newline at end of file

new file mode 100644
index 00000000000..e346deaa1db
@@ -0,0 +1,10 @@
+On branch test
+
+No commits yet
+
+
+It took X seconds to enumerate untracked files.
+See https://git-scm.com/docs/git-status#_untracked_files_and_status_speed
+for configuration options that may improve that time.
+
+nothing to commit (create/copy files and use "git add" to track)

new file mode 100644
index 00000000000..a649d367493
@@ -0,0 +1,11 @@
+On branch test
+
+No commits yet
+
+
+It took X seconds to enumerate untracked files,
+but this is currently being cached, with fsmonitor OFF.
+See https://git-scm.com/docs/git-status#_untracked_files_and_status_speed
+for configuration options that may improve that time.
+
+nothing to commit (create/copy files and use "git add" to track)

new file mode 100644
index 00000000000..d5e95d984f8
@@ -0,0 +1,11 @@
+On branch test
+
+No commits yet
+
+
+It took X seconds to enumerate untracked files,
+but this is currently being cached, with fsmonitor ON.
+See https://git-scm.com/docs/git-status#_untracked_files_and_status_speed
+for configuration options that may improve that time.
+
+nothing to commit (create/copy files and use "git add" to track)

index 5813174896c..be903c6e294 100644
@@ -18,8 +18,10 @@
 #include "worktree.h"
 #include "lockfile.h"
 #include "sequencer.h"
+#include "fsmonitor-settings.h"
 
 #define AB_DELAY_WARNING_IN_MS (2 * 1000)
+#define UF_DELAY_WARNING_IN_MS (2 * 1000)
 
 static const char cut_line[] =
 "------------------------ >8 ------------------------\n";

@@ -1205,6 +1207,17 @@ static void wt_longstatus_print_tracking(struct wt_status *s)
 	strbuf_release(&sb);
 }
 
+static inline int uf_was_slow(uint32_t untracked_in_ms)
+{
+	const char *x;
+	x = getenv("GIT_TEST_UF_DELAY_WARNING");
+	if (x) {
+		untracked_in_ms += UF_DELAY_WARNING_IN_MS + 1;
+	}
+
+	return UF_DELAY_WARNING_IN_MS < untracked_in_ms;
+}
+
 static void show_merge_in_progress(struct wt_status *s,
 				   const char *color)
 {

@@ -1814,6 +1827,7 @@ static void wt_longstatus_print(struct wt_status *s)
 {
 	const char *branch_color = color(WT_STATUS_ONBRANCH, s);
 	const char *branch_status_color = color(WT_STATUS_HEADER, s);
+	enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(s->repo);
 
 	if (s->branch) {
 		const char *on_what = _("On branch ");

@@ -1870,13 +1884,25 @@ static void wt_longstatus_print(struct wt_status *s)
 		wt_longstatus_print_other(s, &s->untracked, _("Untracked files"), "add");
 		if (s->show_ignored_mode)
 			wt_longstatus_print_other(s, &s->ignored, _("Ignored files"), "add -f");
-		if (advice_enabled(ADVICE_STATUS_U_OPTION) && 2000 < s->untracked_in_ms) {
-			status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
-			status_printf_ln(s, GIT_COLOR_NORMAL,
-					 _("It took %.2f seconds to enumerate untracked files. 'status -uno'\n"
-					   "may speed it up, but you have to be careful not to forget to add\n"
-					   "new files yourself (see 'git help status')."),
-					 s->untracked_in_ms / 1000.0);
+		if (uf_was_slow(s->untracked_in_ms)) {
+			if (advice_enabled(ADVICE_STATUS_U_OPTION)) {
+				status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
+				if (s->repo->settings.core_untracked_cache == UNTRACKED_CACHE_WRITE) {
+					status_printf_ln(s, GIT_COLOR_NORMAL,
+							_("It took %.2f seconds to enumerate untracked files,\n"
+							"but this is currently being cached, with fsmonitor %s."),
+							s->untracked_in_ms / 1000.0,
+							(fsm_mode > FSMONITOR_MODE_DISABLED) ? "ON" : "OFF");
+				} else {
+					status_printf_ln(s, GIT_COLOR_NORMAL,
+							_("It took %.2f seconds to enumerate untracked files."),
+							s->untracked_in_ms / 1000.0);
+				}
+				status_printf_ln(s, GIT_COLOR_NORMAL,
+						_("See https://git-scm.com/docs/git-status#_untracked_files_and_status_speed\n"
+						"for configuration options that may improve that time."));
+				status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
+			}
 		}
 	} else if (s->committable)
 		status_printf_ln(s, GIT_COLOR_NORMAL, _("Untracked files not listed%s"),

base-commit: bbe21b64a08f89475d8a3818e20c111378daa621

gitgitgadget

Let me suggest an alternative commit message. We want to lead with a "command" -- as in: "make Git do this" or "teach Git to do this". Then explain why. Maybe something like:

Improve the advice displayed when git status is slow because of excessive numbers of untracked files. Update the git status man page to explain the various configuration options.

git status can be slow when there are a large number of untracked files and directories, because Git must search the entire worktree to enumerate them. Previously, Git would print an advice message with the elapsed search time and a suggestion to disable the search using the -uno option. This suggestion also carried a warning that might scare off some users.

Git can reduce the size and time of the untracked file search when the core.untrackedCache and core.fsmonitor features are enabled by caching results from previous git status invocations.

Update the advice to explain the various combinations of additional configuration options and refer to (new) documentation in the man page that explains it in more detail than what can be printed in an advice message.

Finally, add new tests to verify the new functionality.

Or something like that. :-)

[...]

[...]

[...]

Thanks a lot for all that feedback. Honestly you're way more familiar than I am with both how the underlying features work, and how to best contribute this change, so I'm comfortable taking your advice pretty much blindly.

Oops, sorry for missing this. Well, your commit message suggestion is flawless, I will reuse this as is. Thanks a lot for spending time polishing it.

rewrite.)

There's absolutely nothing to apologize about. This is great! I will also reuse as is. Here too, thanks a lot for the time spent on this!

Sounds good, will fix.

That's all good, I need to align my submission to all this and resubmit anyway, so you got time! I'm pretty confident about them too, a lot more than in the phrasing of the user-facing content I had.

Oops, I didn't realize people were getting the same content from either. I will fix.

That's the one thing I'm a bit concerned about, that I would like to discuss more if that's ok.

The current confusion we're seeing with users of our very large repo, is that they run git status the first time and notice it being slow (~30s), and then they see the current advice message advising that they're supposed to do something about it. What they don't know, is that untrackedcache and fsmonitor were already set for their environment, by the script setting their entire environment up.

I don't think it is unusual for users to not necessarily know how their environment was configured (either because someone/something else did it for them, or because they forgot what they did for this specific repo, for instance).

So with that, I worry about the phrasing "See 'git help status' for information on how to improve this." in that use case, because it implies that there is something they are expected to go improve, while that was already done.

Here are some solution ideas:

* Changing the wording for all use cases to not convey that they must do anything about it. For instance just "See 'git help status'". (I don't love this because I could imagine users being puzzled about why Git is telling them this, then.)
* Informing the user of their current caching situation in ways that they can deduce whether or not they should be doing something about it. That's what I was attempting to do here, but reading your help content, I think I got something wrong: I didn't realize the cache would only need to warm up with untrackedcache + fsmonitor, and not with untrackedcache alone. So with that an improvement could be to only display "but this is currently being cached." when both untrackedcache + fsmonitor are on, and not display anything different when only untrackedcache is on then when it's not.
* Not displaying this advice message when fsmonitor is on, since the best possible optimization was already applied. Not loving it, because some could also still want untracked files off on top of it. Also, it doesn't resolve the frustration of noticing git status being slow the first time.

For now if you don't mind, I'll change things to the 2nd proposal up there, but this is not because I'm rejecting your guidance and insisting that adding a line here is the right solution to the situation of environments that were already optimized, and I'm not sure what is. But I do worry that telling those users that they should optimize things while they/someone already did will surely be confusing.

I'll work on the other fixes, and I am deeply interested in your thoughts about this one. Thanks a lot for working through this with me.

Excellent suggestions, thank you.

I agree with your suggestion of not linking out to git-scm.com here, but I wonder if we could get by without mentioning 'git help' here, either. Presumably looking up an unknown configuration variable with 'man git-config' is easy enough.

Thanks,
Taylor

I'm not strongly opinionated, but I believe the initial idea behind redirecting them to the doc was because Git now comes with more configuration abilities to improve performance of git status, that may be more or less relevant depending on use cases, so there isn't really a single git-config key for them to look up any more. Their ideal solution could be core.untrackedCache=true, core.fsmonitor=true, advice.statusUoption=false, status.showUntrackedFiles=false, or even some combinations of those can be relevant.

From there, the goal I believe we were going for with this new doc section is to let users know what configs exist for their git status slowness pains and why, so they can then go look those configs up for more details, which I agree would indeed be easy from there.

Again, I'm not strongly opinionated, and I hope I accurately represented the inital thinking on this idea.

One slightly stronger opinion I have, is that if the advice message was just

and nothing else, I can definitely see a strong UX downside of not giving a hint of next steps for users. Basically, "you have a problem, and we're not helping you resolve it". Were you thinking more of something like this?

I'd say that's probably somewhat cryptic and a bit verbose (which is what we were trying to avoid by telling them to go see the doc), but we wouldn't be leaving the user stranded, so I can see how that would work out ok.

I'm very interested in what you think.

Thanks,

On the topic in general: I think it's probably a good thing to show the advice, but I just want to point out that it's not without cost.

Right now we're showing users a pretty basic command they can try, but now we're showing them other stuff that needs more complex setup.

For some they're probably way better off, e.g. the untracked cache is pretty much an unambiguous win (we should probably turn it on on default, but we'd need to check on-the-fly if the FS supports it properly).

But for e.g. fsmonitor the user may spend a lot of time fiddling with it, only to find it doesn't help their use-case much, if it all.

Should we still point out these possibilities? Probably, but just say'n.

One thing that I find glaringly omitted, which since you're working on this you might consider adding: Suggest to just try running the exact same command again, maybe it was just the FS cache.

I.e. we're suggesting all this advanced stuff, but by far the biggest difference is made on e.g. a modern *nix box (particularly Linux) by just having all the repo's assets in the FS cache.

I have to admit that would be by far my personal preference.

We've been having a number of very great points made by several people on this thread, but a number of them contradicting each other across people, and yet clearly nobody's wrong, everybody makes very real points. I'm trying to turn this into actionable changes I should make, but I think I need guidance on that. This is my first ever contribution to the project, so I'm lacking the organizational awareness of the project to be able to drive this to a consensus on what we should do.

Here's a proposal that tries to make opinionated moves towards what I understand to be the priorities that were expressed:

1- We keep the new paragraph doc, because I'd say why not, it's well-written (thanks Jeff!) and useful. When people are looking for ways to make git status faster, it's good that there's a reference about it, and I'd expect it to be a common need across all kinds of user situations.

2- When untracked cache is not on, if I understand Ævar's suggestion, it would say something like:

It would satisfy that the message gives concise advice with actionable next steps, without making assumptions about whether it will or won't work. (Untracked cache alone did not make much of a difference in our very large repo's case.) And it doesn't point to the help anymore, in order not to saturate the user with too much detail.

3- When untrackedcache is on but fsmonitor is off, and git status is still slow (that's the situation we had on our very large repo), it could say something like:

Same as before, concise, no assumptions.

This setup is more advanced, but we are in a case where untracked cache is not helping, so I'm thining that should be very few repos. If the user feels a need to better understand what's up, the feature is mentioned by name, so they can look it up and dig in if they wish to.

4- When fsmonitor is on:

Same as before, concise, no assumptions, and matches Ævar's suggestion above that was also my preference, as it would apply perfectly to our very large repo's use case and the grievances we've received.

Please let me know what your thoughts are about it all. A downside with all that is the option to disable untracked files is not mentioned at all, but if we keep the doc as it is, and it gets painful enough that they search for other ways, I'm hopefuly the user would find it there.

I want to say it again: I'm not very opinionated about any of this, just trying to collate feedback into an actionable plan. If I understood feedback wrong, or my plan is not the best based on the feedback, that is very fine, but I will need guidance to know what makes more sense.

I could take on the work to make untracked cache on by default after this, as another patch, if it sounds relevant to try. I feel I lack the technical understanding of what we need to check that you're mentioning here, so I'll have questions, but I'd be on board with trying.

I see what you're saying. On the one hand, it feels redundant to say, "we noticed 'git status' is running slowly for you, try running 'git help status' to figure out why".

But on the other hand, enumerating all possible configuration values that you may or may not want to set given your particular circumstance isn't practical either.

Thinking on it more, I think what you wrote originally is reasonable.

Thanks,
Taylor

From: Rudy Rigot <[email protected]>

Improve the advice displayed when git status is slow because of excessive numbers of untracked files. Update the git status man page to explain the various configuration options.

git status can be slow when there are a large number of untracked files and directories, because Git must search the entire worktree to enumerate them. Previously, Git would print an advice message with the elapsed search time and a suggestion to disable the search using the -uno option. This suggestion also carried a warning that might scare off some users.

Git can reduce the size and time of the untracked file search when the core.untrackedCache and core.fsmonitor features are enabled by caching results from previous git status invocations.

Update the advice to explain the various combinations of additional configuration options and refer to (new) documentation in the man page that explains it in more detail than what can be printed in an advice message.

Finally, add new tests to verify the new functionality.

---
status: long status advice adapted to recent capabilities

Here is version 3 for this patch.

Changes since v2:

* Replaced copy of the commit message with the better one suggested by Jeff.
* Replaced copy of the doc and the default advice message with the better ones suggested by Jeff.
* Fixed EOF.
* Changed the approach for users who are already optimized, pending more conversation to see what makes most sense.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1384%2Frudyrigot%2Fadvice_statusFsmonitor-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1384/rudyrigot/advice_statusFsmonitor-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1384

Range-diff vs v2:

1: 9ef7f1834b7 ! 1: 3c98492cb82 status: long status advice adapted to recent capabilities
@@ Metadata
## Commit message ##
status: long status advice adapted to recent capabilities

- Currently, if git-status takes more than 2 seconds for enumerating untracked
- files, a piece of advice is given to the user to consider ignoring untracked
- files. But Git now offers more possibilities to resolve that situation
- (untracked cache, fsmonitor) with different downsides.
+ Improve the advice displayed when git status is slow because
+ of excessive numbers of untracked files. Update the git status
+ man page to explain the various configuration options.

- This change is about refreshing that advice message. A new section in the
- documentation is introduced to present the possibilities, and the advice
- message links to it. I'm also introducing tests for this advice message,
- which was untested so far.
+ git status can be slow when there are a large number of untracked
+ files and directories, because Git must search the entire worktree
+ to enumerate them. Previously, Git would print an advice message
+ with the elapsed search time and a suggestion to disable the search
+ using the -uno option. This suggestion also carried a warning
+ that might scare off some users.

- One of the downsides of untracked cache / fsmonitor, is that the first call
- may be long in order to generate the cache, but the user may not know what
- their current configuration is. When collecting feedback from users of our
- very large repo, that's the most common point of confusion that keeps coming
- back: people complain about git status being slow, but are satisfied when
- we inform them that it's being cached and they should run it again to check.
- As a result, the advice message tries to keep them informed of their current
- configuration.
+ Git can reduce the size and time of the untracked file search when
+ the core.untrackedCache and core.fsmonitor features are enabled
+ by caching results from previous git status invocations.
+
+ Update the advice to explain the various combinations of additional
+ configuration options and refer to (new) documentation in the man
+ page that explains it in more detail than what can be printed in an
+ advice message.
+
+ Finally, add new tests to verify the new functionality.

@@ Documentation/git-status.txt: during the write may conflict with other simultane
+UNTRACKED FILES AND STATUS SPEED
+--------------------------------
+
-+If your untracked files take an unusual amount of time to enumerate, your -+repository certainly has a lot of them, and an advice message will display -+about it. Here are some configurations to consider in order to improve the -+situation:
-+
-+* Setting the core.untrackedCache configuration as true will allow for -+git status to keep track of the mtime of folders, in order to cache past -+status results and be sure to only browse folders that changed on subsequent -+runs, for filesystems that can support it (see linkgit:git-update-index[1] -+for details).
-+* Used in conjonction with core.untrackedCache, setting the core.fsmonitor -+configuration as true will allow for git status to keep track of what -+files recently changed, in order to cache past status results and be sure -+to only focus on those files on subsequent runs (see linkgit:git-update-index[1] -+for details).
-+* If none of the above options are satisfactory, setting the -+status.showUntrackedFiles configuration as no will cause git status -+to not attempt to list untracked files anymore, in which case you have to be -+careful not to forget to add new files yourself.
-+
-+If none of the above solutions are satisfactory, and you are bothered with -+the advice message, you can disable it by setting the advice.statusUoption -+configuration to false.
++git status can be very slow in large worktrees if/when it
++needs to search for untracked files and directories. There are ++many configuration options available to speed this up by either ++avoiding the work or making use of cached results from previous ++Git commands. Since we all work in different ways, there is no ++single optimum set of settings right for everyone. Here is a ++brief summary of the relevant options to help you choose which
++is right for you. Each of these settings is independently ++documented elsewhere in more detail, so please refer to them
++for complete details.
++
++* -uno or status.showUntrackedFiles=false : just don't search
++ and don't report on untracked files. This is the fastest.
++ git status will not list the untracked files, so you need
++ to be careful to remember if you create any new files and
++ manually git add them.
++
++* advice.statusUoption=false : search, but don't complain if it
++ takes too long.
++
++* core.untrackedCache=true : enable the untracked cache feature
++ and only search directories that have been modified since the
++ previous git status command. Git remembers the set of
++ untracked files within each directory and assumes that if a
++ directory has not been modified, then the set of untracked
++ file within has not changed. This is much faster than
++ enumerating the contents of every directory, but still not
++ without cost, because Git still has to search for the set of
++ modified directories.
++
++* core.untrackedCache=true and core.fsmonitor=true or
++ core.fsmonitor=<hook_command_pathname> : enable both the
++ untracked cache and FSMonitor features and only search
++ directories that have been modified since the previous
++ git status command. This is faster than using just the
++ untracked cache alone because Git can also avoid searching
++ for modified directories. Git only has to enumerate the
++ exact set of directories that have changed recently.
++
++Note that after you turn on the untracked cache and/or FSMonitor ++features it may take a few git status commands for the various ++caches to warm up before you see improved command times. This is ++normal.
+
SEE ALSO
--------
@@ t/t7065-wtstatus-slow.sh (new)
+'
+
+test_done
- \ No newline at end of file

## t/t7065/no_untrackedcache_no_fsmonitor (new) ##
@@
@@ t/t7065/no_untrackedcache_no_fsmonitor (new)
+
+
+It took X seconds to enumerate untracked files.
-+See https://git-scm.com/docs/git-status#_untracked_files_and_status_speed
-+for configuration options that may improve that time.
++See 'git help status' for information on how to improve this.
+
+nothing to commit (create/copy files and use "git add" to track)

@@ t/t7065/with_untrackedcache_no_fsmonitor (new)
+No commits yet
+
+
-+It took X seconds to enumerate untracked files,
-+but this is currently being cached, with fsmonitor OFF. -+See https://git-scm.com/docs/git-status#_untracked_files_and_status_speed
-+for configuration options that may improve that time.
++It took X seconds to enumerate untracked files.
++See 'git help status' for information on how to improve this.
+
+nothing to commit (create/copy files and use "git add" to track)

@@ t/t7065/with_untrackedcache_with_fsmonitor (new)
+
+
+It took X seconds to enumerate untracked files,
-+but this is currently being cached, with fsmonitor ON.
-+See https://git-scm.com/docs/git-status#_untracked_files_and_status_speed
-+for configuration options that may improve that time.
++but this is currently being cached.
++See 'git help status' for information on how to improve this.
+
+nothing to commit (create/copy files and use "git add" to track)

@@ wt-status.c: static void wt_longstatus_print(struct wt_status *s)
+ if (uf_was_slow(s->untracked_in_ms)) {
+ if (advice_enabled(ADVICE_STATUS_U_OPTION)) {
+ status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
-+ if (s->repo->settings.core_untracked_cache == UNTRACKED_CACHE_WRITE) {
++ if (fsm_mode > FSMONITOR_MODE_DISABLED) {
+ status_printf_ln(s, GIT_COLOR_NORMAL,
+ _("It took %.2f seconds to enumerate untracked files,\n"
-+ "but this is currently being cached, with fsmonitor %s."),
-+ s->untracked_in_ms / 1000.0,
-+ (fsm_mode > FSMONITOR_MODE_DISABLED) ? "ON" : "OFF");
++ "but this is currently being cached."),
++ s->untracked_in_ms / 1000.0);
+ } else {
+ status_printf_ln(s, GIT_COLOR_NORMAL,
+ _("It took %.2f seconds to enumerate untracked files."),
+ s->untracked_in_ms / 1000.0);
+ }
+ status_printf_ln(s, GIT_COLOR_NORMAL,
-+ _("See https://git-scm.com/docs/git-status#_untracked_files_and_status_speed\n"
-+ "for configuration options that may improve that time."));
++ _("See 'git help status' for information on how to improve this."));
+ status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
+ }
}

 Documentation/git-status.txt               | 47 ++++++++++++++++++++++
 t/t7065-wtstatus-slow.sh                   | 40 ++++++++++++++++++
 t/t7065/no_untrackedcache_no_fsmonitor     |  9 +++++
 t/t7065/with_untrackedcache_no_fsmonitor   |  9 +++++
 t/t7065/with_untrackedcache_with_fsmonitor | 10 +++++
 wt-status.c                                | 38 +++++++++++++----
 6 files changed, 146 insertions(+), 7 deletions(-)
 create mode 100755 t/t7065-wtstatus-slow.sh
 create mode 100644 t/t7065/no_untrackedcache_no_fsmonitor
 create mode 100644 t/t7065/with_untrackedcache_no_fsmonitor
 create mode 100644 t/t7065/with_untrackedcache_with_fsmonitor

index 54a4b29b473..95f4ed95e96 100644
@@ -457,6 +457,53 @@ during the write may conflict with other simultaneous processes, causing
 them to fail. Scripts running `status` in the background should consider
 using `git --no-optional-locks status` (see linkgit:git[1] for details).
 
+UNTRACKED FILES AND STATUS SPEED
+--------------------------------
+
+`git status` can be very slow in large worktrees if/when it
+needs to search for untracked files and directories.  There are
+many configuration options available to speed this up by either
+avoiding the work or making use of cached results from previous
+Git commands.  Since we all work in different ways, there is no
+single optimum set of settings right for everyone.  Here is a
+brief summary of the relevant options to help you choose which
+is right for you.  Each of these settings is independently
+documented elsewhere in more detail, so please refer to them
+for complete details.
+
+* `-uno` or `status.showUntrackedFiles=false` : just don't search
+    and don't report on untracked files.  This is the fastest.
+    `git status` will not list the untracked files, so you need
+    to be careful to remember if you create any new files and
+    manually `git add` them.
+
+* `advice.statusUoption=false` : search, but don't complain if it
+    takes too long.
+
+* `core.untrackedCache=true` : enable the untracked cache feature
+    and only search directories that have been modified since the
+    previous `git status` command.  Git remembers the set of
+    untracked files within each directory and assumes that if a
+    directory has not been modified, then the set of untracked
+    file within has not changed.  This is much faster than
+    enumerating the contents of every directory, but still not
+    without cost, because Git still has to search for the set of
+    modified directories.
+
+* `core.untrackedCache=true` and `core.fsmonitor=true` or
+    `core.fsmonitor=<hook_command_pathname>` : enable both the
+    untracked cache and FSMonitor features and only search
+    directories that have been modified since the previous
+    `git status` command.  This is faster than using just the
+    untracked cache alone because Git can also avoid searching
+    for modified directories.  Git only has to enumerate the
+    exact set of directories that have changed recently.
+
+Note that after you turn on the untracked cache and/or FSMonitor
+features it may take a few `git status` commands for the various
+caches to warm up before you see improved command times.  This is
+normal.
+
 SEE ALSO
 --------
 linkgit:gitignore[5]

new file mode 100755
index 00000000000..23c37ea71e7
@@ -0,0 +1,40 @@
+#!/bin/sh
+
+test_description='test status when slow untracked files'
+
+. ./test-lib.sh
+
+DATA="$TEST_DIRECTORY/t7065"
+
+GIT_TEST_UF_DELAY_WARNING=1
+export GIT_TEST_UF_DELAY_WARNING
+
+test_expect_success setup '
+	git checkout -b test
+'
+
+test_expect_success 'when core.untrackedCache and fsmonitor are unset' '
+	test_must_fail git config --get core.untrackedCache &&
+	test_must_fail git config --get core.fsmonitor &&
+    git status | sed "s/[0-9]\.[0-9][0-9]/X/g" >../actual &&
+    test_cmp "$DATA/no_untrackedcache_no_fsmonitor" ../actual &&
+    rm -fr ../actual
+'
+
+test_expect_success 'when core.untrackedCache true, but not fsmonitor' '
+    git config core.untrackedCache true &&
+	test_must_fail git config --get core.fsmonitor &&
+    git status | sed "s/[0-9]\.[0-9][0-9]/X/g" >../actual &&
+    test_cmp "$DATA/with_untrackedcache_no_fsmonitor" ../actual &&
+    rm -fr ../actual
+'
+
+test_expect_success 'when core.untrackedCache true, and fsmonitor' '
+    git config core.untrackedCache true &&
+	git config core.fsmonitor true &&
+    git status | sed "s/[0-9]\.[0-9][0-9]/X/g" >../actual &&
+    test_cmp "$DATA/with_untrackedcache_with_fsmonitor" ../actual &&
+    rm -fr ../actual
+'
+
+test_done

new file mode 100644
index 00000000000..91dc3719cda
@@ -0,0 +1,9 @@
+On branch test
+
+No commits yet
+
+
+It took X seconds to enumerate untracked files.
+See 'git help status' for information on how to improve this.
+
+nothing to commit (create/copy files and use "git add" to track)

new file mode 100644
index 00000000000..91dc3719cda
@@ -0,0 +1,9 @@
+On branch test
+
+No commits yet
+
+
+It took X seconds to enumerate untracked files.
+See 'git help status' for information on how to improve this.
+
+nothing to commit (create/copy files and use "git add" to track)

new file mode 100644
index 00000000000..89d2dd5c2e7
@@ -0,0 +1,10 @@
+On branch test
+
+No commits yet
+
+
+It took X seconds to enumerate untracked files,
+but this is currently being cached.
+See 'git help status' for information on how to improve this.
+
+nothing to commit (create/copy files and use "git add" to track)

index 5813174896c..4dfc8a8969b 100644
@@ -18,8 +18,10 @@
 #include "worktree.h"
 #include "lockfile.h"
 #include "sequencer.h"
+#include "fsmonitor-settings.h"
 
 #define AB_DELAY_WARNING_IN_MS (2 * 1000)
+#define UF_DELAY_WARNING_IN_MS (2 * 1000)
 
 static const char cut_line[] =
 "------------------------ >8 ------------------------\n";

@@ -1205,6 +1207,17 @@ static void wt_longstatus_print_tracking(struct wt_status *s)
 	strbuf_release(&sb);
 }
 
+static inline int uf_was_slow(uint32_t untracked_in_ms)
+{
+	const char *x;
+	x = getenv("GIT_TEST_UF_DELAY_WARNING");
+	if (x) {
+		untracked_in_ms += UF_DELAY_WARNING_IN_MS + 1;
+	}
+
+	return UF_DELAY_WARNING_IN_MS < untracked_in_ms;
+}
+
 static void show_merge_in_progress(struct wt_status *s,
 				   const char *color)
 {

@@ -1814,6 +1827,7 @@ static void wt_longstatus_print(struct wt_status *s)
 {
 	const char *branch_color = color(WT_STATUS_ONBRANCH, s);
 	const char *branch_status_color = color(WT_STATUS_HEADER, s);
+	enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(s->repo);
 
 	if (s->branch) {
 		const char *on_what = _("On branch ");

@@ -1870,13 +1884,23 @@ static void wt_longstatus_print(struct wt_status *s)
 		wt_longstatus_print_other(s, &s->untracked, _("Untracked files"), "add");
 		if (s->show_ignored_mode)
 			wt_longstatus_print_other(s, &s->ignored, _("Ignored files"), "add -f");
-		if (advice_enabled(ADVICE_STATUS_U_OPTION) && 2000 < s->untracked_in_ms) {
-			status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
-			status_printf_ln(s, GIT_COLOR_NORMAL,
-					 _("It took %.2f seconds to enumerate untracked files. 'status -uno'\n"
-					   "may speed it up, but you have to be careful not to forget to add\n"
-					   "new files yourself (see 'git help status')."),
-					 s->untracked_in_ms / 1000.0);
+		if (uf_was_slow(s->untracked_in_ms)) {
+			if (advice_enabled(ADVICE_STATUS_U_OPTION)) {
+				status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
+				if (fsm_mode > FSMONITOR_MODE_DISABLED) {
+					status_printf_ln(s, GIT_COLOR_NORMAL,
+							_("It took %.2f seconds to enumerate untracked files,\n"
+							"but this is currently being cached."),
+							s->untracked_in_ms / 1000.0);
+				} else {
+					status_printf_ln(s, GIT_COLOR_NORMAL,
+							_("It took %.2f seconds to enumerate untracked files."),
+							s->untracked_in_ms / 1000.0);
+				}
+				status_printf_ln(s, GIT_COLOR_NORMAL,
+						_("See 'git help status' for information on how to improve this."));
+				status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
+			}
 		}
 	} else if (s->committable)
 		status_printf_ln(s, GIT_COLOR_NORMAL, _("Untracked files not listed%s"),

base-commit: bbe21b64a08f89475d8a3818e20c111378daa621

gitgitgadget

This one is looking good to me. Jeff: do you agree? If so, I'm ready to start merging this one down.

Thanks,
Taylor

Sorry, I came in with some late review. I think one more round would be helpful.

Thanks,
-Stolee

Thanks, all. Will wait for one more round before we start merging this down.

Thanks,
Taylor

This series is looking good to me too. I think V5 has addressed all of the concerns.

Jeff

On 11/2/22 5:27 PM, Rudy Rigot via GitGitGadget wrote:> +UNTRACKED FILES AND STATUS SPEED

Two nits:

1. Is it clear that -uno is an option to git status? Should we say "The -uno flag or the status.showUntrackedfiles=false config"?

2. Drop the "just" as it implies simplicity but is unnecessary. In fact, I'd replace the sentence with:

Indicate that git status should not report untracked files.

"This is the fastest option." ?

	cat >expect <<-\EOF
	<file contents here>
	EOF &&

I've been meaning to respond to say the exact same thing about using here-doc instead.

Also, the new tests seem to have an oddball mixture of indentation using spaces and TAB. They should be using TAB only.

Thanks to both of you Derrick and Eric, this is all great feedback. I've been through all of it, it's all crystal clear, and I'm planning to just integrate all of it as is. I'll have a new iteration later this week (it's small stuff, I know, but I'm volunteering as a poll worker today and tomorrow).

Thanks again!

From: Rudy Rigot <[email protected]>

Improve the advice displayed when git status is slow because of excessive numbers of untracked files. Update the git status man page to explain the various configuration options.

git status can be slow when there are a large number of untracked files and directories, because Git must search the entire worktree to enumerate them. Previously, Git would print an advice message with the elapsed search time and a suggestion to disable the search using the -uno option. This suggestion also carried a warning that might scare off some users.

Git can reduce the size and time of the untracked file search when the core.untrackedCache and core.fsmonitor features are enabled by caching results from previous git status invocations.

Update the advice to explain the various combinations of additional configuration options and refer to (new) documentation in the man page that explains it in more detail than what can be printed in an advice message.

Finally, add new tests to verify the new functionality.

---
status: long status advice adapted to recent capabilities

Here is version 4 for this patch.

Changes since v3:

* Integrated all feedback on the doc content itself, as is.
* Moved the small test files into here docs in the test code.
* Fix some awkward indentations.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1384%2Frudyrigot%2Fadvice_statusFsmonitor-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1384/rudyrigot/advice_statusFsmonitor-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1384

Range-diff vs v3:

1: 3c98492cb82 ! 1: 85b35882c02 status: long status advice adapted to recent capabilities

     @@ Documentation/git-status.txt: during the write may conflict with other simultane
      +documented elsewhere in more detail, so please refer to them
      +for complete details.
      +
     -+* `-uno` or `status.showUntrackedFiles=false` : just don't search
     -+    and don't report on untracked files.  This is the fastest.
     -+    `git status` will not list the untracked files, so you need
     -+    to be careful to remember if you create any new files and
     -+    manually `git add` them.
     ++* The `-uno` flag or the `status.showUntrackedfiles=false`
     ++    config : indicate that `git status` should not report untracked
     ++	files. This is the fastest option. `git status` will not list
     ++	the untracked files, so you need to be careful to remember if
     ++	you create any new files and manually `git add` them.
      +
     -+* `advice.statusUoption=false` : search, but don't complain if it
     -+    takes too long.
     ++* `advice.statusUoption=false` : this config option disables a
     ++	warning message when the search for untracked files takes longer
     ++	than desired. In some large repositories, this message may appear
     ++	frequently and not be a helpful signal.
      +
      +* `core.untrackedCache=true` : enable the untracked cache feature
      +    and only search directories that have been modified since the
     @@ Documentation/git-status.txt: during the write may conflict with other simultane
      +    file within has not changed.  This is much faster than
      +    enumerating the contents of every directory, but still not
      +    without cost, because Git still has to search for the set of
     -+    modified directories.
     ++    modified directories. The untracked cache is stored in the
     ++	.git/index file. The reduced cost searching for untracked
     ++	files is offset slightly by the increased size of the index and
     ++	the cost of keeping it up-to-date. That reduced search time is
     ++	usually worth the additional size.
      +
      +* `core.untrackedCache=true` and `core.fsmonitor=true` or
      +    `core.fsmonitor=<hook_command_pathname>` : enable both the
     @@ Documentation/git-status.txt: during the write may conflict with other simultane
      +    `git status` command.  This is faster than using just the
      +    untracked cache alone because Git can also avoid searching
      +    for modified directories.  Git only has to enumerate the
     -+    exact set of directories that have changed recently.
     ++    exact set of directories that have changed recently. While
     ++	the FSMonitor feature can be enabled without the untracked
     ++	cache, the benefits are greatly reduced in that case.
      +
      +Note that after you turn on the untracked cache and/or FSMonitor
      +features it may take a few `git status` commands for the various
     @@ t/t7065-wtstatus-slow.sh (new)
      +test_expect_success 'when core.untrackedCache and fsmonitor are unset' '
      +	test_must_fail git config --get core.untrackedCache &&
      +	test_must_fail git config --get core.fsmonitor &&
     -+    git status | sed "s/[0-9]\.[0-9][0-9]/X/g" >../actual &&
     -+    test_cmp "$DATA/no_untrackedcache_no_fsmonitor" ../actual &&
     -+    rm -fr ../actual
     -+'
     -+
     -+test_expect_success 'when core.untrackedCache true, but not fsmonitor' '
     -+    git config core.untrackedCache true &&
     -+	test_must_fail git config --get core.fsmonitor &&
     -+    git status | sed "s/[0-9]\.[0-9][0-9]/X/g" >../actual &&
     -+    test_cmp "$DATA/with_untrackedcache_no_fsmonitor" ../actual &&
     -+    rm -fr ../actual
     -+'
     -+
     -+test_expect_success 'when core.untrackedCache true, and fsmonitor' '
     -+    git config core.untrackedCache true &&
     -+	git config core.fsmonitor true &&
     -+    git status | sed "s/[0-9]\.[0-9][0-9]/X/g" >../actual &&
     -+    test_cmp "$DATA/with_untrackedcache_with_fsmonitor" ../actual &&
     -+    rm -fr ../actual
     -+'
     -+
     -+test_done
     -
     - ## t/t7065/no_untrackedcache_no_fsmonitor (new) ##
     -@@
     ++	git status | sed "s/[0-9]\.[0-9][0-9]/X/g" >../actual &&
     ++	cat >../expected <<-EOF &&
      +On branch test
      +
      +No commits yet
      +
      +
      +It took X seconds to enumerate untracked files.
     -+See 'git help status' for information on how to improve this.
     ++See '"'"'git help status'"'"' for information on how to improve this.
      +
      +nothing to commit (create/copy files and use "git add" to track)
     -
     - ## t/t7065/with_untrackedcache_no_fsmonitor (new) ##
     -@@
     ++	EOF
     ++	test_cmp ../expected ../actual &&
     ++	rm -fr ../actual ../expected
     ++'
     ++
     ++test_expect_success 'when core.untrackedCache true, but not fsmonitor' '
     ++	git config core.untrackedCache true &&
     ++	test_must_fail git config --get core.fsmonitor &&
     ++	git status | sed "s/[0-9]\.[0-9][0-9]/X/g" >../actual &&
     ++	cat >../expected <<-EOF &&
      +On branch test
      +
      +No commits yet
      +
      +
      +It took X seconds to enumerate untracked files.
     -+See 'git help status' for information on how to improve this.
     ++See '"'"'git help status'"'"' for information on how to improve this.
      +
      +nothing to commit (create/copy files and use "git add" to track)
     -
     - ## t/t7065/with_untrackedcache_with_fsmonitor (new) ##
     -@@
     ++	EOF
     ++	test_cmp ../expected ../actual &&
     ++	rm -fr ../actual ../expected
     ++'
     ++
     ++test_expect_success 'when core.untrackedCache true, and fsmonitor' '
     ++	git config core.untrackedCache true &&
     ++	git config core.fsmonitor true &&
     ++	git status | sed "s/[0-9]\.[0-9][0-9]/X/g" >../actual &&
     ++	cat >../expected <<-EOF &&
      +On branch test
      +
      +No commits yet
     @@ t/t7065/with_untrackedcache_with_fsmonitor (new)
      +
      +It took X seconds to enumerate untracked files,
      +but this is currently being cached.
     -+See 'git help status' for information on how to improve this.
     ++See '"'"'git help status'"'"' for information on how to improve this.
      +
      +nothing to commit (create/copy files and use "git add" to track)
     ++	EOF
     ++	test_cmp ../expected ../actual &&
     ++	rm -fr ../actual ../expected
     ++'
     ++
     ++test_done
      
       ## wt-status.c ##
      @@
     @@ wt-status.c: static void wt_longstatus_print(struct wt_status *s)
       		if (s->show_ignored_mode)
       			wt_longstatus_print_other(s, &s->ignored, _("Ignored files"), "add -f");
      -		if (advice_enabled(ADVICE_STATUS_U_OPTION) && 2000 < s->untracked_in_ms) {
     --			status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
     --			status_printf_ln(s, GIT_COLOR_NORMAL,
     ++		if (uf_was_slow(s->untracked_in_ms) && advice_enabled(ADVICE_STATUS_U_OPTION)) {
     + 			status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
     ++			if (fsm_mode > FSMONITOR_MODE_DISABLED) {
     ++				status_printf_ln(s, GIT_COLOR_NORMAL,
     ++						_("It took %.2f seconds to enumerate untracked files,\n"
     ++						"but this is currently being cached."),
     ++						s->untracked_in_ms / 1000.0);
     ++			} else {
     ++				status_printf_ln(s, GIT_COLOR_NORMAL,
     ++						_("It took %.2f seconds to enumerate untracked files."),
     ++						s->untracked_in_ms / 1000.0);
     ++			}
     + 			status_printf_ln(s, GIT_COLOR_NORMAL,
      -					 _("It took %.2f seconds to enumerate untracked files. 'status -uno'\n"
      -					   "may speed it up, but you have to be careful not to forget to add\n"
      -					   "new files yourself (see 'git help status')."),
      -					 s->untracked_in_ms / 1000.0);
     -+		if (uf_was_slow(s->untracked_in_ms)) {
     -+			if (advice_enabled(ADVICE_STATUS_U_OPTION)) {
     -+				status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
     -+				if (fsm_mode > FSMONITOR_MODE_DISABLED) {
     -+					status_printf_ln(s, GIT_COLOR_NORMAL,
     -+							_("It took %.2f seconds to enumerate untracked files,\n"
     -+							"but this is currently being cached."),
     -+							s->untracked_in_ms / 1000.0);
     -+				} else {
     -+					status_printf_ln(s, GIT_COLOR_NORMAL,
     -+							_("It took %.2f seconds to enumerate untracked files."),
     -+							s->untracked_in_ms / 1000.0);
     -+				}
     -+				status_printf_ln(s, GIT_COLOR_NORMAL,
     -+						_("See 'git help status' for information on how to improve this."));
     -+				status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
     -+			}
     ++					_("See 'git help status' for information on how to improve this."));
     ++			status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
       		}
       	} else if (s->committable)
       		status_printf_ln(s, GIT_COLOR_NORMAL, _("Untracked files not listed%s"),

 Documentation/git-status.txt | 55 +++++++++++++++++++++++++++
 t/t7065-wtstatus-slow.sh     | 74 ++++++++++++++++++++++++++++++++++++
 wt-status.c                  | 32 +++++++++++++---
 3 files changed, 156 insertions(+), 5 deletions(-)
 create mode 100755 t/t7065-wtstatus-slow.sh

index 54a4b29b473..c51ba5e79e1 100644
@@ -457,6 +457,61 @@ during the write may conflict with other simultaneous processes, causing
 them to fail. Scripts running `status` in the background should consider
 using `git --no-optional-locks status` (see linkgit:git[1] for details).
 
+UNTRACKED FILES AND STATUS SPEED
+--------------------------------
+
+`git status` can be very slow in large worktrees if/when it
+needs to search for untracked files and directories.  There are
+many configuration options available to speed this up by either
+avoiding the work or making use of cached results from previous
+Git commands.  Since we all work in different ways, there is no
+single optimum set of settings right for everyone.  Here is a
+brief summary of the relevant options to help you choose which
+is right for you.  Each of these settings is independently
+documented elsewhere in more detail, so please refer to them
+for complete details.
+
+* The `-uno` flag or the `status.showUntrackedfiles=false`
+    config : indicate that `git status` should not report untracked
+	files. This is the fastest option. `git status` will not list
+	the untracked files, so you need to be careful to remember if
+	you create any new files and manually `git add` them.
+
+* `advice.statusUoption=false` : this config option disables a
+	warning message when the search for untracked files takes longer
+	than desired. In some large repositories, this message may appear
+	frequently and not be a helpful signal.
+
+* `core.untrackedCache=true` : enable the untracked cache feature
+    and only search directories that have been modified since the
+    previous `git status` command.  Git remembers the set of
+    untracked files within each directory and assumes that if a
+    directory has not been modified, then the set of untracked
+    file within has not changed.  This is much faster than
+    enumerating the contents of every directory, but still not
+    without cost, because Git still has to search for the set of
+    modified directories. The untracked cache is stored in the
+	.git/index file. The reduced cost searching for untracked
+	files is offset slightly by the increased size of the index and
+	the cost of keeping it up-to-date. That reduced search time is
+	usually worth the additional size.
+
+* `core.untrackedCache=true` and `core.fsmonitor=true` or
+    `core.fsmonitor=<hook_command_pathname>` : enable both the
+    untracked cache and FSMonitor features and only search
+    directories that have been modified since the previous
+    `git status` command.  This is faster than using just the
+    untracked cache alone because Git can also avoid searching
+    for modified directories.  Git only has to enumerate the
+    exact set of directories that have changed recently. While
+	the FSMonitor feature can be enabled without the untracked
+	cache, the benefits are greatly reduced in that case.
+
+Note that after you turn on the untracked cache and/or FSMonitor
+features it may take a few `git status` commands for the various
+caches to warm up before you see improved command times.  This is
+normal.
+
 SEE ALSO
 --------
 linkgit:gitignore[5]

index 5813174896c..336f41e6d9f 100644
@@ -18,8 +18,10 @@
 #include "worktree.h"
 #include "lockfile.h"
 #include "sequencer.h"
+#include "fsmonitor-settings.h"
 
 #define AB_DELAY_WARNING_IN_MS (2 * 1000)
+#define UF_DELAY_WARNING_IN_MS (2 * 1000)
 
 static const char cut_line[] =
 "------------------------ >8 ------------------------\n";

@@ -1205,6 +1207,17 @@ static void wt_longstatus_print_tracking(struct wt_status *s)
 	strbuf_release(&sb);
 }
 
+static inline int uf_was_slow(uint32_t untracked_in_ms)
+{
+	const char *x;
+	x = getenv("GIT_TEST_UF_DELAY_WARNING");
+	if (x) {
+		untracked_in_ms += UF_DELAY_WARNING_IN_MS + 1;
+	}
+
+	return UF_DELAY_WARNING_IN_MS < untracked_in_ms;
+}
+
 static void show_merge_in_progress(struct wt_status *s,
 				   const char *color)
 {

@@ -1814,6 +1827,7 @@ static void wt_longstatus_print(struct wt_status *s)
 {
 	const char *branch_color = color(WT_STATUS_ONBRANCH, s);
 	const char *branch_status_color = color(WT_STATUS_HEADER, s);
+	enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(s->repo);
 
 	if (s->branch) {
 		const char *on_what = _("On branch ");

@@ -1870,13 +1884,21 @@ static void wt_longstatus_print(struct wt_status *s)
 		wt_longstatus_print_other(s, &s->untracked, _("Untracked files"), "add");
 		if (s->show_ignored_mode)
 			wt_longstatus_print_other(s, &s->ignored, _("Ignored files"), "add -f");
-		if (advice_enabled(ADVICE_STATUS_U_OPTION) && 2000 < s->untracked_in_ms) {
+		if (uf_was_slow(s->untracked_in_ms) && advice_enabled(ADVICE_STATUS_U_OPTION)) {
 			status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
+			if (fsm_mode > FSMONITOR_MODE_DISABLED) {
+				status_printf_ln(s, GIT_COLOR_NORMAL,
+						_("It took %.2f seconds to enumerate untracked files,\n"
+						"but this is currently being cached."),
+						s->untracked_in_ms / 1000.0);
+			} else {
+				status_printf_ln(s, GIT_COLOR_NORMAL,
+						_("It took %.2f seconds to enumerate untracked files."),
+						s->untracked_in_ms / 1000.0);
+			}
 			status_printf_ln(s, GIT_COLOR_NORMAL,
-					 _("It took %.2f seconds to enumerate untracked files. 'status -uno'\n"
-					   "may speed it up, but you have to be careful not to forget to add\n"
-					   "new files yourself (see 'git help status')."),
-					 s->untracked_in_ms / 1000.0);
+					_("See 'git help status' for information on how to improve this."));
+			status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
 		}
 	} else if (s->committable)
 		status_printf_ln(s, GIT_COLOR_NORMAL, _("Untracked files not listed%s"),

base-commit: bbe21b64a08f89475d8a3818e20c111378daa621

gitgitgadget

Unfortunately, that's how I had done it first, but since some of those lines are blank, the test code had lines just made of "<tab><tab>" and nothing else, which made the check-whitespace check fail. I considered replacing empty line with something on the fly with sed (like just an "x" character for instance), but this felt hacky and brittle (in the unlikely case where an actual "x" would find itself genuinely lost in the middle of that output, the test would mistakenly pass). I went with the solution I'm presenting here because the readability downsides of missing that indentation felt less bad. Definitely willing to be convinced though.

I have no issues with anything else from your review, and I'm planning to integrate it all. More specifically:

You are presuming right! The test was being flappy in CI runs before I changed this, which I found used as a solution in other git-status-related tests currently in the codebase. I'm not familiar with the trash directory approach, but I'll figure it out.

I'll admit my limits, I'm not familiar enough to know. If you feel the inline is unnecessary here, I'm glad to trust you on it, I'll remove it.

Thanks a lot for your in-depth review, I am planning to integrate all the other feedback, and bring another iteration forward, possibly today.

Okay, I see what you're getting at. Fortunately, there is a simple solution as long as those lines are truly blank as emitted by git status: just leave the blank lines completely blank in the here-doc body (don't bother inserting a TAB on the blank line). This should product the exact output you want:

cat >../expected <<-\EOF &&
On branch test

No commits yet
...
EOF

Although it should not be needed here, the sed approach is generally fine, and we use it often enough in tests, though usually with a more uncommon letter such as "Q". See, for instance, the q_to_nul(), q_to_tab(), etc. functions in t/test-lib-functions.sh.

Each test script is run in a temporary "trash" directory which gets thrown away when the script finishes. We want tests to constrain themselves to the trash directory so they don't inadvertently destroy a user's files outside the directory.

I see what you mean about some existing status-related tests using files such as "../actual" and "../expect". It's not at all obvious in a lot of those cases but they are safe[*] because those tests have already cd'd into a subdirectory of the "trash" directory, thus "../actual" is referring to the "trash" directory itself, hence the tests do constrain themselves to "trash".

Anyhow, I suspect that crafting a custom .gitignore file in the test setup should satisfy this particular case and allow "actual" and "expected" to reside in the "trash" directory itself without mucking up git status output.

[*] Unfortunately, some of those scripts are poorly structured because they cd around between tests, which can leave CWD in an unexpected state if some test fails and subsequent tests expect CWD to be somewhere other than where it was left by the failed test. These days, we only allow tests to cd within a subshell so that CWD is restored automatically whether the test itself succeeds or fails. So, this is safe:

    test_expect_success 'title' '
        do something &&
        mkdir foo &&
        (
            cd foo &&
            do something else >../actual &&
        )
        grep foo actual
    '

Oh, thanks, all of this helps a ton. I know exactly what to do about those two things now.

From: Rudy Rigot <[email protected]>

Improve the advice displayed when git status is slow because of excessive numbers of untracked files. Update the git status man page to explain the various configuration options.

git status can be slow when there are a large number of untracked files and directories, because Git must search the entire worktree to enumerate them. Previously, Git would print an advice message with the elapsed search time and a suggestion to disable the search using the -uno option. This suggestion also carried a warning that might scare off some users.

Git can reduce the size and time of the untracked file search when the core.untrackedCache and core.fsmonitor features are enabled by caching results from previous git status invocations.

Update the advice to explain the various combinations of additional configuration options and refer to (new) documentation in the man page that explains it in more detail than what can be printed in an advice message.

Finally, add new tests to verify the new functionality.

---
status: long status advice adapted to recent capabilities

Here is version 5 for this patch.

Changes since v4:

* Test improvements (readability, isolation. ...)
* Doc improvements (referencing other docs, adding advice to try again, ...)
* Minor logic simplifications

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1384%2Frudyrigot%2Fadvice_statusFsmonitor-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1384/rudyrigot/advice_statusFsmonitor-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/1384

Range-diff vs v4:

1: 85b35882c02 ! 1: 8b1b9ee094f status: long status advice adapted to recent capabilities
@@ Documentation/git-status.txt: during the write may conflict with other simultane
+--------------------------------
+
+git status can be very slow in large worktrees if/when it
-+needs to search for untracked files and directories. There are ++needs to search for untracked files and directories. There are
+many configuration options available to speed this up by either +avoiding the work or making use of cached results from previous
-+Git commands. Since we all work in different ways, there is no -+single optimum set of settings right for everyone. Here is a
-+brief summary of the relevant options to help you choose which
-+is right for you. Each of these settings is independently -+documented elsewhere in more detail, so please refer to them
-+for complete details.
-+
-+* The -uno flag or the status.showUntrackedfiles=false
-+ config : indicate that git status should not report untracked
++Git commands. There is no single optimum set of settings right
++for everyone. Here is a brief summary of the relevant options
++to help you choose which is right for you.
++
++* First, you may want to run git status again. Your current
++ configuration may already be caching git status results,
++ so it could be faster on subsequent runs.
++
++* The --untracked-files=no flag or the
++ status.showUntrackedfiles=false config (see above for both) :
++ indicate that git status should not report untracked
+ files. This is the fastest option. git status will not list
+ the untracked files, so you need to be careful to remember if
+ you create any new files and manually git add them.
+
-+* advice.statusUoption=false : this config option disables a
-+ warning message when the search for untracked files takes longer
-+ than desired. In some large repositories, this message may appear
-+ frequently and not be a helpful signal.
-+
-+* core.untrackedCache=true : enable the untracked cache feature
-+ and only search directories that have been modified since the
-+ previous git status command. Git remembers the set of
-+ untracked files within each directory and assumes that if a
-+ directory has not been modified, then the set of untracked
-+ file within has not changed. This is much faster than
-+ enumerating the contents of every directory, but still not
-+ without cost, because Git still has to search for the set of
-+ modified directories. The untracked cache is stored in the
++* advice.statusUoption=false (see linkgit:git-config[1]) :
++ this config option disables a warning message when the search
++ for untracked files takes longer than desired. In some large
++ repositories, this message may appear frequently and not be a
++ helpful signal.
++
++* core.untrackedCache=true (see linkgit:git-update-index[1]) :
++ enable the untracked cache feature and only search directories
++ that have been modified since the previous git status command.
++ Git remembers the set of untracked files within each directory
++ and assumes that if a directory has not been modified, then
++ the set of untracked file within has not changed. This is much
++ faster than enumerating the contents of every directory, but still
++ not without cost, because Git still has to search for the set of
++ modified directories. The untracked cache is stored in the
+ .git/index file. The reduced cost searching for untracked
+ files is offset slightly by the increased size of the index and
+ the cost of keeping it up-to-date. That reduced search time is
+ usually worth the additional size.
+
+* core.untrackedCache=true and core.fsmonitor=true or
-+ core.fsmonitor=<hook_command_pathname> : enable both the
-+ untracked cache and FSMonitor features and only search
-+ directories that have been modified since the previous
-+ git status command. This is faster than using just the
-+ untracked cache alone because Git can also avoid searching
-+ for modified directories. Git only has to enumerate the
-+ exact set of directories that have changed recently. While
-+ the FSMonitor feature can be enabled without the untracked
-+ cache, the benefits are greatly reduced in that case.
++ core.fsmonitor=<hook_command_pathname> (see
++ linkgit:git-update-index[1]) : enable both the untracked cache
++ and FSMonitor features and only search directories that have
++ been modified since the previous git status command. This
++ is faster than using just the untracked cache alone because
++ Git can also avoid searching for modified directories. Git
++ only has to enumerate the exact set of directories that have
++ changed recently. While the FSMonitor feature can be enabled
++ without the untracked cache, the benefits are greatly reduced
++ in that case.
+
+Note that after you turn on the untracked cache and/or FSMonitor +features it may take a few git status commands for the various
@@ t/t7065-wtstatus-slow.sh (new)
+
+. ./test-lib.sh
+
-+DATA="$TEST_DIRECTORY/t7065"
-+
+GIT_TEST_UF_DELAY_WARNING=1
+export GIT_TEST_UF_DELAY_WARNING
+
+test_expect_success setup '
-+ git checkout -b test
++ git checkout -b test &&
++ echo "actual" >> .gitignore &&
++ echo "expected" >> .gitignore &&
++ echo "out" >> .gitignore &&
++ git add .gitignore &&
++ git commit -m "Add .gitignore"
+'
+
+test_expect_success 'when core.untrackedCache and fsmonitor are unset' '
-+ test_must_fail git config --get core.untrackedCache &&
-+ test_must_fail git config --get core.fsmonitor &&
-+ git status | sed "s/[0-9]\.[0-9][0-9]/X/g" >../actual &&
-+ cat >../expected <<-EOF &&
-+On branch test
-+
-+No commits yet
-+
++ test_might_fail git config --unset-all core.untrackedCache &&
++ test_might_fail git config --unset-all core.fsmonitor &&
++ git status >out &&
++ sed "s/[0-9]\.[0-9][0-9]/X/g" out >actual &&
++ cat >expected <<-\EOF &&
++ On branch test
+
-+It took X seconds to enumerate untracked files.
-+See '"'"'git help status'"'"' for information on how to improve this.
++ It took X seconds to enumerate untracked files.
++ See '"'"'git help status'"'"' for information on how to improve this.
+
-+nothing to commit (create/copy files and use "git add" to track)
++ nothing to commit, working tree clean
+ EOF
-+ test_cmp ../expected ../actual &&
-+ rm -fr ../actual ../expected
++ test_cmp expected actual
+'
+
+test_expect_success 'when core.untrackedCache true, but not fsmonitor' '
+ git config core.untrackedCache true &&
-+ test_must_fail git config --get core.fsmonitor &&
-+ git status | sed "s/[0-9]\.[0-9][0-9]/X/g" >../actual &&
-+ cat >../expected <<-EOF &&
-+On branch test
++ test_might_fail git config --unset-all core.fsmonitor &&
++ git status >out &&
++ sed "s/[0-9]\.[0-9][0-9]/X/g" out >actual &&
++ cat >expected <<-\EOF &&
++ On branch test
+
-+No commits yet
++ It took X seconds to enumerate untracked files.
++ See '"'"'git help status'"'"' for information on how to improve this.
+
-+
-+It took X seconds to enumerate untracked files.
-+See '"'"'git help status'"'"' for information on how to improve this. -+
-+nothing to commit (create/copy files and use "git add" to track)
++ nothing to commit, working tree clean
+ EOF
-+ test_cmp ../expected ../actual &&
-+ rm -fr ../actual ../expected
++ test_cmp expected actual
+'
+
+test_expect_success 'when core.untrackedCache true, and fsmonitor' '
+ git config core.untrackedCache true &&
+ git config core.fsmonitor true &&
-+ git status | sed "s/[0-9]\.[0-9][0-9]/X/g" >../actual &&
-+ cat >../expected <<-EOF &&
-+On branch test
-+
-+No commits yet
-+
++ git status >out &&
++ sed "s/[0-9]\.[0-9][0-9]/X/g" out >actual &&
++ cat >expected <<-\EOF &&
++ On branch test
+
-+It took X seconds to enumerate untracked files,
-+but this is currently being cached.
-+See '"'"'git help status'"'"' for information on how to improve this.
++ It took X seconds to enumerate untracked files,
++ but this is currently being cached.
++ See '"'"'git help status'"'"' for information on how to improve this.
+
-+nothing to commit (create/copy files and use "git add" to track)
++ nothing to commit, working tree clean
+ EOF
-+ test_cmp ../expected ../actual &&
-+ rm -fr ../actual ../expected
++ test_cmp expected actual
+'
+
+test_done
@@ wt-status.c: static void wt_longstatus_print_tracking(struct wt_status *s)

+static inline int uf_was_slow(uint32_t untracked_in_ms) +{
-+ const char *x;
-+ x = getenv("GIT_TEST_UF_DELAY_WARNING");
-+ if (x) {
++ if (getenv("GIT_TEST_UF_DELAY_WARNING")) {
+ untracked_in_ms += UF_DELAY_WARNING_IN_MS + 1;
+ }
+

 Documentation/git-status.txt | 59 +++++++++++++++++++++++++++++++
 t/t7065-wtstatus-slow.sh     | 68 ++++++++++++++++++++++++++++++++++++
 wt-status.c                  | 30 +++++++++++++---
 3 files changed, 152 insertions(+), 5 deletions(-)
 create mode 100755 t/t7065-wtstatus-slow.sh

index 5e438a7fdc1..ed1ae3bd35c 100644
@@ -457,6 +457,65 @@ during the write may conflict with other simultaneous processes, causing
 them to fail. Scripts running `status` in the background should consider
 using `git --no-optional-locks status` (see linkgit:git[1] for details).
 
+UNTRACKED FILES AND STATUS SPEED
+--------------------------------
+
+`git status` can be very slow in large worktrees if/when it
+needs to search for untracked files and directories. There are
+many configuration options available to speed this up by either
+avoiding the work or making use of cached results from previous
+Git commands. There is no single optimum set of settings right
+for everyone.  Here is a brief summary of the relevant options
+to help you choose which is right for you.
+
+* First, you may want to run `git status` again. Your current
+	configuration may already be caching `git status` results,
+	so it could be faster on subsequent runs.
+
+* The `--untracked-files=no` flag or the
+	`status.showUntrackedfiles=false` config (see above for both) :
+	indicate that `git status` should not report untracked
+	files. This is the fastest option. `git status` will not list
+	the untracked files, so you need to be careful to remember if
+	you create any new files and manually `git add` them.
+
+* `advice.statusUoption=false` (see linkgit:git-config[1]) :
+	this config option disables a warning message when the search
+	for untracked files takes longer than desired. In some large
+	repositories, this message may appear frequently and not be a
+	helpful signal.
+
+* `core.untrackedCache=true` (see linkgit:git-update-index[1]) :
+	enable the untracked cache feature and only search directories
+	that have been modified since the previous `git status` command.
+	Git remembers the set of untracked files within each directory
+	and assumes that if a directory has not been modified, then
+	the set of untracked file within has not changed.  This is much
+	faster than enumerating the contents of every directory, but still
+	not without cost, because Git still has to search for the set of
+	modified directories. The untracked cache is stored in the
+	.git/index file. The reduced cost searching for untracked
+	files is offset slightly by the increased size of the index and
+	the cost of keeping it up-to-date. That reduced search time is
+	usually worth the additional size.
+
+* `core.untrackedCache=true` and `core.fsmonitor=true` or
+	`core.fsmonitor=<hook_command_pathname>` (see
+	linkgit:git-update-index[1]) : enable both the untracked cache
+	and FSMonitor features and only search directories that have
+	been modified since the previous `git status` command.  This
+	is faster than using just the untracked cache alone because
+	Git can also avoid searching for modified directories.  Git
+	only has to enumerate the exact set of directories that have
+	changed recently. While the FSMonitor feature can be enabled
+	without the untracked cache, the benefits are greatly reduced
+	in that case.
+
+Note that after you turn on the untracked cache and/or FSMonitor
+features it may take a few `git status` commands for the various
+caches to warm up before you see improved command times.  This is
+normal.
+
 SEE ALSO
 --------
 linkgit:gitignore[5]

index 5813174896c..5093bf8c894 100644
@@ -18,8 +18,10 @@
 #include "worktree.h"
 #include "lockfile.h"
 #include "sequencer.h"
+#include "fsmonitor-settings.h"
 
 #define AB_DELAY_WARNING_IN_MS (2 * 1000)
+#define UF_DELAY_WARNING_IN_MS (2 * 1000)
 
 static const char cut_line[] =
 "------------------------ >8 ------------------------\n";

@@ -1205,6 +1207,15 @@ static void wt_longstatus_print_tracking(struct wt_status *s)
 	strbuf_release(&sb);
 }
 
+static inline int uf_was_slow(uint32_t untracked_in_ms)
+{
+	if (getenv("GIT_TEST_UF_DELAY_WARNING")) {
+		untracked_in_ms += UF_DELAY_WARNING_IN_MS + 1;
+	}
+
+	return UF_DELAY_WARNING_IN_MS < untracked_in_ms;
+}
+
 static void show_merge_in_progress(struct wt_status *s,
 				   const char *color)
 {

@@ -1814,6 +1825,7 @@ static void wt_longstatus_print(struct wt_status *s)
 {
 	const char *branch_color = color(WT_STATUS_ONBRANCH, s);
 	const char *branch_status_color = color(WT_STATUS_HEADER, s);
+	enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(s->repo);
 
 	if (s->branch) {
 		const char *on_what = _("On branch ");

@@ -1870,13 +1882,21 @@ static void wt_longstatus_print(struct wt_status *s)
 		wt_longstatus_print_other(s, &s->untracked, _("Untracked files"), "add");
 		if (s->show_ignored_mode)
 			wt_longstatus_print_other(s, &s->ignored, _("Ignored files"), "add -f");
-		if (advice_enabled(ADVICE_STATUS_U_OPTION) && 2000 < s->untracked_in_ms) {
+		if (uf_was_slow(s->untracked_in_ms) && advice_enabled(ADVICE_STATUS_U_OPTION)) {
 			status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
+			if (fsm_mode > FSMONITOR_MODE_DISABLED) {
+				status_printf_ln(s, GIT_COLOR_NORMAL,
+						_("It took %.2f seconds to enumerate untracked files,\n"
+						"but this is currently being cached."),
+						s->untracked_in_ms / 1000.0);
+			} else {
+				status_printf_ln(s, GIT_COLOR_NORMAL,
+						_("It took %.2f seconds to enumerate untracked files."),
+						s->untracked_in_ms / 1000.0);
+			}
 			status_printf_ln(s, GIT_COLOR_NORMAL,
-					 _("It took %.2f seconds to enumerate untracked files. 'status -uno'\n"
-					   "may speed it up, but you have to be careful not to forget to add\n"
-					   "new files yourself (see 'git help status')."),
-					 s->untracked_in_ms / 1000.0);
+					_("See 'git help status' for information on how to improve this."));
+			status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
 		}
 	} else if (s->committable)
 		status_printf_ln(s, GIT_COLOR_NORMAL, _("Untracked files not listed%s"),

base-commit: 319605f8f00e402f3ea758a02c63534ff800a711

gitgitgadget

I think V5 looks good.
Thanks for your persistence!

Jeff

I confirm I don't think there was any concern expressed so far that I didn't find a way to address in some way.

My absolute pleasure!

Mostly just some minor style-related comments below, but also a couple grammos in the new documentation, and a question or two...

    echo "actual" >>.gitignore &&
    echo "expected" >>.gitignore &&
    echo "out" >>.gitignore &&

    cat >.gitignore <<-\EOF &&
    actual
    expected
    out
    EOF

    cat >.gitignore <<-\EOF &&
    /actual
    /expected
    /out
    EOF

    cat >expected <<-\EOF &&
    On branch test
    ...
    EOF

    test_config core.untrackedCache true &&

    if (getenv("GIT_TEST_UF_DELAY_WARNING"))
        untracked_in_ms += UF_DELAY_WARNING_IN_MS + 1;
    return UF_DELAY_WARNING_IN_MS < untracked_in_ms;

Thanks for the feedback, and I'll integrate it into a new patch, most likely today.

Oops. Just as I said I don't think I left any feedback unaddressed. You did mention it last time, and it fell through the cracks. I'll fix it this time, sorry about that.

Totally a whim. After several iterations, the code had changed enough that the original ordering was lost. I'll switch it back.

Hah, good point. The accurate answer is "the status of currently existing files is being cached, and we'll watch what files changed after now so we only run things on those next time". Obviously that would be too verbose for the inexperienced user hitting this, really this line is here to convey "if you run it again, it's probably going to be faster".

Here are some ideas:
- "but this result is currently being cached."
- "but git status results are currently being cached." (true but not perfectly accurate since index updates don't only happen on git status)
- "but untracked files are currently being cached." (not completely accurate, I believe the index is updated for all files; the untracked files are only the interesting ones for this specific performance consideration)
- "but the results were cached, and your next runs may be faster."

I could use some guidance on what would make most sense here. I strongly feel like the user should know of it since that's been what's been confusing the users of our very large repo specifically when their git status is temporarily slow; but I don't have any opinions at all about the right phrasing. For now, I'm planning to use the latter bullet point in my next patch because it's the most explicit, but I'd be glad to apply someone else's take on this instead.

Thanks. I think this is _almost_ there; much more polished than earlier iterations. Hopefully, one final reroll will make it complete.

Reading the proposals while wearing the hat of someone who has never had to deal with speeding up untracked-file bookkeeping and who may not even know that remedies are available (as discussed in the documentation), I find that the first three bullet points convey no meaning at all; they leave the reader hanging. The final bullet point, on the other hand, tells the user something conclusive. I _very_ much prefer the final proposal.

(In "but the results were cached, and your next runs may be faster.", I might suggest dropping "your" -- i.e. "... and subsequent runs may be faster".)

That makes a ton of sense, I agree with your thinking. Thanks for that, I'll do exactly that.

From: Rudy Rigot <[email protected]>

Improve the advice displayed when git status is slow because of excessive numbers of untracked files. Update the git status man page to explain the various configuration options.

git status can be slow when there are a large number of untracked files and directories, because Git must search the entire worktree to enumerate them. Previously, Git would print an advice message with the elapsed search time and a suggestion to disable the search using the -uno option. This suggestion also carried a warning that might scare off some users.

Git can reduce the size and time of the untracked file search when the core.untrackedCache and core.fsmonitor features are enabled by caching results from previous git status invocations.

Update the advice to explain the various combinations of additional configuration options and refer to (new) documentation in the man page that explains it in more detail than what can be printed in an advice message.

Finally, add new tests to verify the new functionality.

---
status: long status advice adapted to recent capabilities

Here is version 6 for this patch.

Changes since v5:

* End of sentence for fsmonitor case was changed to "but the results were cached, and subsequent runs may be faster."
* Except for that, mostly style and doc fixes.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1384%2Frudyrigot%2Fadvice_statusFsmonitor-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1384/rudyrigot/advice_statusFsmonitor-v6
Pull-Request: https://github.com/gitgitgadget/git/pull/1384

Range-diff vs v5:

1: 8b1b9ee094f ! 1: ff3aa0e01c0 status: long status advice adapted to recent capabilities

     @@ Documentation/git-status.txt: during the write may conflict with other simultane
      +	that have been modified since the previous `git status` command.
      +	Git remembers the set of untracked files within each directory
      +	and assumes that if a directory has not been modified, then
     -+	the set of untracked file within has not changed.  This is much
     ++	the set of untracked files within has not changed.  This is much
      +	faster than enumerating the contents of every directory, but still
      +	not without cost, because Git still has to search for the set of
      +	modified directories. The untracked cache is stored in the
     -+	.git/index file. The reduced cost searching for untracked
     ++	`.git/index` file. The reduced cost of searching for untracked
      +	files is offset slightly by the increased size of the index and
      +	the cost of keeping it up-to-date. That reduced search time is
      +	usually worth the additional size.
     @@ t/t7065-wtstatus-slow.sh (new)
      +
      +test_expect_success setup '
      +	git checkout -b test &&
     -+	echo "actual" >> .gitignore &&
     -+	echo "expected" >> .gitignore &&
     -+	echo "out" >> .gitignore &&
     ++	cat >.gitignore <<-\EOF &&
     ++	/actual
     ++	/expected
     ++	/out
     ++	EOF
      +	git add .gitignore &&
      +	git commit -m "Add .gitignore"
      +'
     @@ t/t7065-wtstatus-slow.sh (new)
      +	git status >out &&
      +	sed "s/[0-9]\.[0-9][0-9]/X/g" out >actual &&
      +	cat >expected <<-\EOF &&
     -+		On branch test
     ++	On branch test
      +
     -+		It took X seconds to enumerate untracked files.
     -+		See '"'"'git help status'"'"' for information on how to improve this.
     ++	It took X seconds to enumerate untracked files.
     ++	See '"'"'git help status'"'"' for information on how to improve this.
      +
     -+		nothing to commit, working tree clean
     ++	nothing to commit, working tree clean
      +	EOF
      +	test_cmp expected actual
      +'
      +
      +test_expect_success 'when core.untrackedCache true, but not fsmonitor' '
     -+	git config core.untrackedCache true &&
     ++	test_config core.untrackedCache true &&
      +	test_might_fail git config --unset-all core.fsmonitor &&
      +	git status >out &&
      +	sed "s/[0-9]\.[0-9][0-9]/X/g" out >actual &&
      +	cat >expected <<-\EOF &&
     -+		On branch test
     ++	On branch test
      +
     -+		It took X seconds to enumerate untracked files.
     -+		See '"'"'git help status'"'"' for information on how to improve this.
     ++	It took X seconds to enumerate untracked files.
     ++	See '"'"'git help status'"'"' for information on how to improve this.
      +
     -+		nothing to commit, working tree clean
     ++	nothing to commit, working tree clean
      +	EOF
      +	test_cmp expected actual
      +'
      +
      +test_expect_success 'when core.untrackedCache true, and fsmonitor' '
     -+	git config core.untrackedCache true &&
     -+	git config core.fsmonitor true &&
     ++	test_config core.untrackedCache true &&
     ++	test_config core.fsmonitor true &&
      +	git status >out &&
      +	sed "s/[0-9]\.[0-9][0-9]/X/g" out >actual &&
      +	cat >expected <<-\EOF &&
     -+		On branch test
     ++	On branch test
      +
     -+		It took X seconds to enumerate untracked files,
     -+		but this is currently being cached.
     -+		See '"'"'git help status'"'"' for information on how to improve this.
     ++	It took X seconds to enumerate untracked files,
     ++	but the results were cached, and subsequent runs may be faster.
     ++	See '"'"'git help status'"'"' for information on how to improve this.
      +
     -+		nothing to commit, working tree clean
     ++	nothing to commit, working tree clean
      +	EOF
      +	test_cmp expected actual
      +'
     @@ wt-status.c: static void wt_longstatus_print_tracking(struct wt_status *s)
       	strbuf_release(&sb);
       }
       
     -+static inline int uf_was_slow(uint32_t untracked_in_ms)
     ++static int uf_was_slow(uint32_t untracked_in_ms)
      +{
     -+	if (getenv("GIT_TEST_UF_DELAY_WARNING")) {
     ++	if (getenv("GIT_TEST_UF_DELAY_WARNING"))
      +		untracked_in_ms += UF_DELAY_WARNING_IN_MS + 1;
     -+	}
     -+
      +	return UF_DELAY_WARNING_IN_MS < untracked_in_ms;
      +}
      +
     @@ wt-status.c: static void wt_longstatus_print(struct wt_status *s)
       		if (s->show_ignored_mode)
       			wt_longstatus_print_other(s, &s->ignored, _("Ignored files"), "add -f");
      -		if (advice_enabled(ADVICE_STATUS_U_OPTION) && 2000 < s->untracked_in_ms) {
     -+		if (uf_was_slow(s->untracked_in_ms) && advice_enabled(ADVICE_STATUS_U_OPTION)) {
     ++		if (advice_enabled(ADVICE_STATUS_U_OPTION) && uf_was_slow(s->untracked_in_ms)) {
       			status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
      +			if (fsm_mode > FSMONITOR_MODE_DISABLED) {
      +				status_printf_ln(s, GIT_COLOR_NORMAL,
      +						_("It took %.2f seconds to enumerate untracked files,\n"
     -+						"but this is currently being cached."),
     ++						"but the results were cached, and subsequent runs may be faster."),
      +						s->untracked_in_ms / 1000.0);
      +			} else {
      +				status_printf_ln(s, GIT_COLOR_NORMAL,

 Documentation/git-status.txt | 59 ++++++++++++++++++++++++++++++
 t/t7065-wtstatus-slow.sh     | 70 ++++++++++++++++++++++++++++++++++++
 wt-status.c                  | 28 ++++++++++++---
 3 files changed, 152 insertions(+), 5 deletions(-)
 create mode 100755 t/t7065-wtstatus-slow.sh

index 5e438a7fdc1..570c36e07c1 100644
@@ -457,6 +457,65 @@ during the write may conflict with other simultaneous processes, causing
 them to fail. Scripts running `status` in the background should consider
 using `git --no-optional-locks status` (see linkgit:git[1] for details).
 
+UNTRACKED FILES AND STATUS SPEED
+--------------------------------
+
+`git status` can be very slow in large worktrees if/when it
+needs to search for untracked files and directories. There are
+many configuration options available to speed this up by either
+avoiding the work or making use of cached results from previous
+Git commands. There is no single optimum set of settings right
+for everyone.  Here is a brief summary of the relevant options
+to help you choose which is right for you.
+
+* First, you may want to run `git status` again. Your current
+	configuration may already be caching `git status` results,
+	so it could be faster on subsequent runs.
+
+* The `--untracked-files=no` flag or the
+	`status.showUntrackedfiles=false` config (see above for both) :
+	indicate that `git status` should not report untracked
+	files. This is the fastest option. `git status` will not list
+	the untracked files, so you need to be careful to remember if
+	you create any new files and manually `git add` them.
+
+* `advice.statusUoption=false` (see linkgit:git-config[1]) :
+	this config option disables a warning message when the search
+	for untracked files takes longer than desired. In some large
+	repositories, this message may appear frequently and not be a
+	helpful signal.
+
+* `core.untrackedCache=true` (see linkgit:git-update-index[1]) :
+	enable the untracked cache feature and only search directories
+	that have been modified since the previous `git status` command.
+	Git remembers the set of untracked files within each directory
+	and assumes that if a directory has not been modified, then
+	the set of untracked files within has not changed.  This is much
+	faster than enumerating the contents of every directory, but still
+	not without cost, because Git still has to search for the set of
+	modified directories. The untracked cache is stored in the
+	`.git/index` file. The reduced cost of searching for untracked
+	files is offset slightly by the increased size of the index and
+	the cost of keeping it up-to-date. That reduced search time is
+	usually worth the additional size.
+
+* `core.untrackedCache=true` and `core.fsmonitor=true` or
+	`core.fsmonitor=<hook_command_pathname>` (see
+	linkgit:git-update-index[1]) : enable both the untracked cache
+	and FSMonitor features and only search directories that have
+	been modified since the previous `git status` command.  This
+	is faster than using just the untracked cache alone because
+	Git can also avoid searching for modified directories.  Git
+	only has to enumerate the exact set of directories that have
+	changed recently. While the FSMonitor feature can be enabled
+	without the untracked cache, the benefits are greatly reduced
+	in that case.
+
+Note that after you turn on the untracked cache and/or FSMonitor
+features it may take a few `git status` commands for the various
+caches to warm up before you see improved command times.  This is
+normal.
+
 SEE ALSO
 --------
 linkgit:gitignore[5]

index 5813174896c..1f6d64e759f 100644
@@ -18,8 +18,10 @@
 #include "worktree.h"
 #include "lockfile.h"
 #include "sequencer.h"
+#include "fsmonitor-settings.h"
 
 #define AB_DELAY_WARNING_IN_MS (2 * 1000)
+#define UF_DELAY_WARNING_IN_MS (2 * 1000)
 
 static const char cut_line[] =
 "------------------------ >8 ------------------------\n";

@@ -1205,6 +1207,13 @@ static void wt_longstatus_print_tracking(struct wt_status *s)
 	strbuf_release(&sb);
 }
 
+static int uf_was_slow(uint32_t untracked_in_ms)
+{
+	if (getenv("GIT_TEST_UF_DELAY_WARNING"))
+		untracked_in_ms += UF_DELAY_WARNING_IN_MS + 1;
+	return UF_DELAY_WARNING_IN_MS < untracked_in_ms;
+}
+
 static void show_merge_in_progress(struct wt_status *s,
 				   const char *color)
 {

@@ -1814,6 +1823,7 @@ static void wt_longstatus_print(struct wt_status *s)
 {
 	const char *branch_color = color(WT_STATUS_ONBRANCH, s);
 	const char *branch_status_color = color(WT_STATUS_HEADER, s);
+	enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(s->repo);
 
 	if (s->branch) {
 		const char *on_what = _("On branch ");

@@ -1870,13 +1880,21 @@ static void wt_longstatus_print(struct wt_status *s)
 		wt_longstatus_print_other(s, &s->untracked, _("Untracked files"), "add");
 		if (s->show_ignored_mode)
 			wt_longstatus_print_other(s, &s->ignored, _("Ignored files"), "add -f");
-		if (advice_enabled(ADVICE_STATUS_U_OPTION) && 2000 < s->untracked_in_ms) {
+		if (advice_enabled(ADVICE_STATUS_U_OPTION) && uf_was_slow(s->untracked_in_ms)) {
 			status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
+			if (fsm_mode > FSMONITOR_MODE_DISABLED) {
+				status_printf_ln(s, GIT_COLOR_NORMAL,
+						_("It took %.2f seconds to enumerate untracked files,\n"
+						"but the results were cached, and subsequent runs may be faster."),
+						s->untracked_in_ms / 1000.0);
+			} else {
+				status_printf_ln(s, GIT_COLOR_NORMAL,
+						_("It took %.2f seconds to enumerate untracked files."),
+						s->untracked_in_ms / 1000.0);
+			}
 			status_printf_ln(s, GIT_COLOR_NORMAL,
-					 _("It took %.2f seconds to enumerate untracked files. 'status -uno'\n"
-					   "may speed it up, but you have to be careful not to forget to add\n"
-					   "new files yourself (see 'git help status')."),
-					 s->untracked_in_ms / 1000.0);
+					_("See 'git help status' for information on how to improve this."));
+			status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
 		}
 	} else if (s->committable)
 		status_printf_ln(s, GIT_COLOR_NORMAL, _("Untracked files not listed%s"),

base-commit: 319605f8f00e402f3ea758a02c63534ff800a711

gitgitgadget

Thanks, I think v6 addresses all my earlier review comments. Just one or two notes below...

    test_unconfig core.untrackedCache &&
    test_unconfig core.fsmonitor &&

Eh, there's nothing wrong with striving for perfection. Lemme do one more reroll...

The reason was that it failed some CI pipelines before I did this, with some pipelines printing "main" instead of "master" into the git status output. I fixed it right away, so I don't know if it was a CI glitch that day or if it would still be the same running it now. I could have redacted the branch name away from the output, but it seemed simpler and more readable to just set the branch name in stone for all pipelines.

Oh, this seems like a better way to do what I was trying to do. I'll change it now.

I'll use that.

New patch coming!

Reviewer time is a scarce resource on the mailing list these days, which is why I'm hesitant to see rerolls for minor or subjective changes. However, if you're going to reroll anyhow, I have a couple more things to say... (below)

Most likely it wasn't a glitch, but rather (I'd guess) that Windows CI uses "main" already, whereas Unix CI's still use "master".

If you're going to reroll, then I'll mention a couple more things which I held back before since I want to use reviewer time wisely. Nevertheless...

First, having the commit message explain the problem first and then the solution is more reviewer-friendly, not the solution and then the problem as this patch is doing. Additionally, the commit message should be written in imperative mood. Documentation/SubmittingPatches has a good discussion of these points. It's also typically unnecessary for the commit message to say that the patch is adding new tests; reviewers assume that you will do so when appropriate, and the patch itself shows plainly enough that you did. Taking these points into consideration, you might write the commit message like this:

status: modernize git-status "slow untracked files" advice

git status can be slow when there are a large number of untracked files and directories since Git must search the entire worktree to enumerate them. When it is too slow, Git prints advice with the elapsed search time and a suggestion to disable the search using the -uno option. This suggestion also carries a warning that might scare off some users.

However, these days, -uno isn't the only option. Git can reduce the size and time of the untracked file search when the core.untrackedCache and core.fsmonitor features are enabled by caching results from previous git status invocations.

Therefore, update the git status man page to explain the various configuration options, and update the advice to provide more detail about the current configuration and to refer to the updated documentation.

Second, we usually don't want to waste a test script number (such as "t7065") if we can avoid it, especially for so few tests and such minor functionality. So, if there is an existing test script in which these new tests might fit, it's better to add them to that script instead, usually at the end of the script. (I haven't checked, but maybe you can find an existing script which would be a good fit; if not, then placing them in a new standalone script, as the patch is already doing, may be okay.)

My intuition was that it was something like that indeed.

The current phrasing was initially copied as is from a past review feedback; I have no issues at all replacing it with yours.

Oh, sorry about that, it hadn't occurred to me that there could be a downside to using new test script numbers.

I'm 100% on board with the thinking, but I'm struggling quite a bit to implement it. There are several existing test scripts where these new tests would fit very well semantically (t7060-wtstatus.sh,c, t7512-status-help.sh, t7519-status-fsmonitor.sh, ...), and I spent quite some time yesterday trying to move the 3 news tests to those. For some reason, test_cmp is not giving me a diff anymore when working in those script files, so I feel in the dark about what the tests are failing about, and I'm stumped about what to try next.

What I mean: for instance, if I introduce an intentional mistake in the test and run './t7065-wtstatus-slow.sh -v', I get this section that clarifies what the issue is:

@@ -1,4 +1,4 @@
-On branch maine
+On branch main
 

Here is a gist https://gist.github.com/rudyrigot/b31fcb6384e829ca7586818758e48d0b,
with:

- the patch as I currently have it on t7508-status.sh (it's a bit longer than it was, without the isolation in a separate script I've had to do a few things to mitigate the side effects from other tests in the script)
- the end of what I get when running 'sh ./t7508-status.sh -v':
https://gist.github.com/rudyrigot/ee80f3d59231f25698c9dd6c48d8ab85. It seems like 2 of my 3 tests are failing, but the output isn't very helpful to figure out why.

Would you (or someone else) have pointers to help me get through this one?

I'm tempted to throw in the towel, since it sounded like it wasn't too huge a deal if this lived in its separate script file, and that other people's bandwidth (which I'm aware is what I'm requesting here) is an even more scarce resource. So I'll submit a new patch with everything else but this, so there's the option to still proceed with it if that's the most sensible path forward. But I have to admit I'm quite frustrated that I couldn't figure this last one out by myself, so I'm more than happy to dig more into it, if anyone has guidance.

Thanks a lot.

Apparently, I overlooked that when scanning backward through the thread.

No need to apologize. Reviewers understand that there is a lot to absorb and figure out.

I probably should have thought to warn about possible interaction with earlier tests when I made the suggestion to place the tests in an existing script. Nevertheless, we should be able to sidestep all that "isolation goop" you had to add to the tests... (more below).

The second 'gist' URL seems broken, so I can't comment on the exact output. Without having seen the actual problem, I can't really provide direct feedback for fixing the precise issue, however...

We actually don't want you to pollute your new tests with goop to clean up after earlier tests in the script since that just introduces a bunch of code in your tests which is not directly relevant to what your tests are checking. So, perhaps the cleanest way to approach this is to have your "setup" test create a new repository in the trash directory and then just run your remaining new tests in that repository. That way, your tests don't have to deal with any gunk from earlier tests. This basically means taking your tests pretty much as you had them in v6, but with a little extra boilerplate. Something like this:

    test_expect_success setup '
        git init slowstatus &&
        (
            cd slowstatus &&
            cat >.gitignore <<-\EOF &&
            /actual
            /expected
            /out
           EOF
           git add .gitignore &&
           git commit -m "Add .gitignore"
        )
    '

    test_expect_success 'when core.untrackedCache and fsmonitor are unset' '
        (
            cd slowstatus &&
            test_unconfig core.untrackedCache &&
            test_unconfig core.fsmonitor &&
            ...
        )
    '

and so on.

My comment about possibly leaving them in a separate script if you couldn't find a suitable existing script was just general advice. I can't speak for Junio and thus can't say what he will or will not accept in his tree.

Perhaps the suggestion I outlined above will help.

Despite the URL of the second git being broken, I notice that you have some "-v" output tacked onto the first gist. Indeed, the "-v" output isn't very helpful here. What I normally do when debugging failing tests is run with "-x" and "-i". The "-x" shows all the output the test produces, including any error messages. So "./t7508-status.sh -x -i" would likely make it easier to diagnose such failures. (But first try the suggestion I made in my previous reply for isolating the new tests in their own repository.)

Oops, here is the only Gist URL I had meant to share, sorry about the confusion, the other one was a copy-paste mistake:
https://gist.github.com/rudyrigot/b31fcb6384e829ca7586818758e48d0b

But the suggestions you just gave me are wonderfully helpful! Thanks a lot for that, let me give those a try now...

A minor additional comment if you do go this route and place the new tests in an existing script...

Although "setup" was fine as a title in a standalone script, when adding the new tests to an existing script, you'd probably want to choose a more meaningful name. Perhaps "setup slow untracked-cache status" or something.

And one more comment...

By placing:

GIT_TEST_UF_DELAY_WARNING=1
export GIT_TEST_UF_DELAY_WARNING

at the top of the existing script into which you add the new tests, we have to worry about potential side-effects in other tests in the scripts. Better would be to place these lines just above the new tests, so that the effects are better isolated. However, even better than that would be to isolate the environment variable to exactly the point it is needed. For instance:

    test_expect_success 'when core.untrackedCache and fsmonitor are unset' '
       test_unconfig core.untrackedCache &&
       test_unconfig core.fsmonitor &&
       GIT_TEST_UF_DELAY_WARNING=1 git status >out &&
       ...
    '

Holy snap it worked! Patch coming as soon as CI confirms it's good everywhere. I believe I was able to integrate all of your advice. Thanks a lot for your guidance!

Glad to hear that it all worked out.

From: Rudy Rigot <[email protected]>

git status can be slow when there are a large number of untracked files and directories since Git must search the entire worktree to enumerate them. When it is too slow, Git prints advice with the elapsed search time and a suggestion to disable the search using the -uno option. This suggestion also carries a warning that might scare off some users.

However, these days, -uno isn't the only option. Git can reduce the size and time of the untracked file search when the core.untrackedCache and core.fsmonitor features are enabled by caching results from previous git status invocations.

Therefore, update the git status man page to explain the various configuration options, and update the advice to provide more detail about the current configuration and to refer to the updated documentation.

---
status: modernize git-status "slow untracked files" advice

Here is version 7 for this patch.

Changes since v6:

* Readability improvements in tests
* Rewrite of the commit message

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1384%2Frudyrigot%2Fadvice_statusFsmonitor-v7
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1384/rudyrigot/advice_statusFsmonitor-v7
Pull-Request: https://github.com/gitgitgadget/git/pull/1384

Range-diff vs v6:

1: ff3aa0e01c0 ! 1: 871a9becbdf status: long status advice adapted to recent capabilities
@@ Metadata
Author: Rudy Rigot <[email protected]>

## Commit message ##
- status: long status advice adapted to recent capabilities
+ status: modernize git-status "slow untracked files" advice

- Improve the advice displayed when git status is slow because
- of excessive numbers of untracked files. Update the git status
- man page to explain the various configuration options.
+ git status can be slow when there are a large number of
+ untracked files and directories since Git must search the entire
+ worktree to enumerate them. When it is too slow, Git prints
+ advice with the elapsed search time and a suggestion to disable
+ the search using the -uno option. This suggestion also carries
+ a warning that might scare off some users.

- git status can be slow when there are a large number of untracked
- files and directories, because Git must search the entire worktree
- to enumerate them. Previously, Git would print an advice message
- with the elapsed search time and a suggestion to disable the search
- using the -uno option. This suggestion also carried a warning
- that might scare off some users.
+ However, these days, -uno isn't the only option. Git can reduce
+ the size and time of the untracked file search when the
+ core.untrackedCache and core.fsmonitor features are enabled by
+ caching results from previous git status invocations.

- Git can reduce the size and time of the untracked file search when
- the core.untrackedCache and core.fsmonitor features are enabled
- by caching results from previous git status invocations.
-
- Update the advice to explain the various combinations of additional
- configuration options and refer to (new) documentation in the man
- page that explains it in more detail than what can be printed in an
- advice message.
-
- Finally, add new tests to verify the new functionality.
+ Therefore, update the git status man page to explain the various
+ configuration options, and update the advice to provide more
+ detail about the current configuration and to refer to the updated
+ documentation.

@@ t/t7065-wtstatus-slow.sh (new)
+
+test_description='test status when slow untracked files'
+
-+. ./test-lib.sh
++GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
++export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+GIT_TEST_UF_DELAY_WARNING=1
+export GIT_TEST_UF_DELAY_WARNING
+
++. ./test-lib.sh
++
+test_expect_success setup '
-+ git checkout -b test &&
+ cat >.gitignore <<-\EOF &&
+ /actual
+ /expected
@@ t/t7065-wtstatus-slow.sh (new)
+'
+
+test_expect_success 'when core.untrackedCache and fsmonitor are unset' '
-+ test_might_fail git config --unset-all core.untrackedCache &&
-+ test_might_fail git config --unset-all core.fsmonitor &&
++ test_unconfig core.untrackedCache &&
++ test_unconfig core.fsmonitor &&
+ git status >out &&
+ sed "s/[0-9]\.[0-9][0-9]/X/g" out >actual &&
+ cat >expected <<-\EOF &&
-+ On branch test
++ On branch main
+
+ It took X seconds to enumerate untracked files.
+ See '"'"'git help status'"'"' for information on how to improve this.
@@ t/t7065-wtstatus-slow.sh (new)
+
+test_expect_success 'when core.untrackedCache true, but not fsmonitor' '
+ test_config core.untrackedCache true &&
-+ test_might_fail git config --unset-all core.fsmonitor &&
++ test_unconfig core.fsmonitor &&
+ git status >out &&
+ sed "s/[0-9]\.[0-9][0-9]/X/g" out >actual &&
+ cat >expected <<-\EOF &&
-+ On branch test
++ On branch main
+
+ It took X seconds to enumerate untracked files.
+ See '"'"'git help status'"'"' for information on how to improve this.
@@ t/t7065-wtstatus-slow.sh (new)
+ git status >out &&
+ sed "s/[0-9]\.[0-9][0-9]/X/g" out >actual &&
+ cat >expected <<-\EOF &&
-+ On branch test
++ On branch main
+
+ It took X seconds to enumerate untracked files,
+ but the results were cached, and subsequent runs may be faster.

 Documentation/git-status.txt | 59 +++++++++++++++++++++++++++++
 t/t7065-wtstatus-slow.sh     | 72 ++++++++++++++++++++++++++++++++++++
 wt-status.c                  | 28 +++++++++++---
 3 files changed, 154 insertions(+), 5 deletions(-)
 create mode 100755 t/t7065-wtstatus-slow.sh

index 5e438a7fdc1..570c36e07c1 100644
@@ -457,6 +457,65 @@ during the write may conflict with other simultaneous processes, causing
 them to fail. Scripts running `status` in the background should consider
 using `git --no-optional-locks status` (see linkgit:git[1] for details).
 
+UNTRACKED FILES AND STATUS SPEED
+--------------------------------
+
+`git status` can be very slow in large worktrees if/when it
+needs to search for untracked files and directories. There are
+many configuration options available to speed this up by either
+avoiding the work or making use of cached results from previous
+Git commands. There is no single optimum set of settings right
+for everyone.  Here is a brief summary of the relevant options
+to help you choose which is right for you.
+
+* First, you may want to run `git status` again. Your current
+	configuration may already be caching `git status` results,
+	so it could be faster on subsequent runs.
+
+* The `--untracked-files=no` flag or the
+	`status.showUntrackedfiles=false` config (see above for both) :
+	indicate that `git status` should not report untracked
+	files. This is the fastest option. `git status` will not list
+	the untracked files, so you need to be careful to remember if
+	you create any new files and manually `git add` them.
+
+* `advice.statusUoption=false` (see linkgit:git-config[1]) :
+	this config option disables a warning message when the search
+	for untracked files takes longer than desired. In some large
+	repositories, this message may appear frequently and not be a
+	helpful signal.
+
+* `core.untrackedCache=true` (see linkgit:git-update-index[1]) :
+	enable the untracked cache feature and only search directories
+	that have been modified since the previous `git status` command.
+	Git remembers the set of untracked files within each directory
+	and assumes that if a directory has not been modified, then
+	the set of untracked files within has not changed.  This is much
+	faster than enumerating the contents of every directory, but still
+	not without cost, because Git still has to search for the set of
+	modified directories. The untracked cache is stored in the
+	`.git/index` file. The reduced cost of searching for untracked
+	files is offset slightly by the increased size of the index and
+	the cost of keeping it up-to-date. That reduced search time is
+	usually worth the additional size.
+
+* `core.untrackedCache=true` and `core.fsmonitor=true` or
+	`core.fsmonitor=<hook_command_pathname>` (see
+	linkgit:git-update-index[1]) : enable both the untracked cache
+	and FSMonitor features and only search directories that have
+	been modified since the previous `git status` command.  This
+	is faster than using just the untracked cache alone because
+	Git can also avoid searching for modified directories.  Git
+	only has to enumerate the exact set of directories that have
+	changed recently. While the FSMonitor feature can be enabled
+	without the untracked cache, the benefits are greatly reduced
+	in that case.
+
+Note that after you turn on the untracked cache and/or FSMonitor
+features it may take a few `git status` commands for the various
+caches to warm up before you see improved command times.  This is
+normal.
+
 SEE ALSO
 --------
 linkgit:gitignore[5]

index 5813174896c..1f6d64e759f 100644
@@ -18,8 +18,10 @@
 #include "worktree.h"
 #include "lockfile.h"
 #include "sequencer.h"
+#include "fsmonitor-settings.h"
 
 #define AB_DELAY_WARNING_IN_MS (2 * 1000)
+#define UF_DELAY_WARNING_IN_MS (2 * 1000)
 
 static const char cut_line[] =
 "------------------------ >8 ------------------------\n";

@@ -1205,6 +1207,13 @@ static void wt_longstatus_print_tracking(struct wt_status *s)
 	strbuf_release(&sb);
 }
 
+static int uf_was_slow(uint32_t untracked_in_ms)
+{
+	if (getenv("GIT_TEST_UF_DELAY_WARNING"))
+		untracked_in_ms += UF_DELAY_WARNING_IN_MS + 1;
+	return UF_DELAY_WARNING_IN_MS < untracked_in_ms;
+}
+
 static void show_merge_in_progress(struct wt_status *s,
 				   const char *color)
 {

@@ -1814,6 +1823,7 @@ static void wt_longstatus_print(struct wt_status *s)
 {
 	const char *branch_color = color(WT_STATUS_ONBRANCH, s);
 	const char *branch_status_color = color(WT_STATUS_HEADER, s);
+	enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(s->repo);
 
 	if (s->branch) {
 		const char *on_what = _("On branch ");

@@ -1870,13 +1880,21 @@ static void wt_longstatus_print(struct wt_status *s)
 		wt_longstatus_print_other(s, &s->untracked, _("Untracked files"), "add");
 		if (s->show_ignored_mode)
 			wt_longstatus_print_other(s, &s->ignored, _("Ignored files"), "add -f");
-		if (advice_enabled(ADVICE_STATUS_U_OPTION) && 2000 < s->untracked_in_ms) {
+		if (advice_enabled(ADVICE_STATUS_U_OPTION) && uf_was_slow(s->untracked_in_ms)) {
 			status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
+			if (fsm_mode > FSMONITOR_MODE_DISABLED) {
+				status_printf_ln(s, GIT_COLOR_NORMAL,
+						_("It took %.2f seconds to enumerate untracked files,\n"
+						"but the results were cached, and subsequent runs may be faster."),
+						s->untracked_in_ms / 1000.0);
+			} else {
+				status_printf_ln(s, GIT_COLOR_NORMAL,
+						_("It took %.2f seconds to enumerate untracked files."),
+						s->untracked_in_ms / 1000.0);
+			}
 			status_printf_ln(s, GIT_COLOR_NORMAL,
-					 _("It took %.2f seconds to enumerate untracked files. 'status -uno'\n"
-					   "may speed it up, but you have to be careful not to forget to add\n"
-					   "new files yourself (see 'git help status')."),
-					 s->untracked_in_ms / 1000.0);
+					_("See 'git help status' for information on how to improve this."));
+			status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
 		}
 	} else if (s->committable)
 		status_printf_ln(s, GIT_COLOR_NORMAL, _("Untracked files not listed%s"),

base-commit: 319605f8f00e402f3ea758a02c63534ff800a711

gitgitgadget

From: Rudy Rigot <[email protected]>

git status can be slow when there are a large number of untracked files and directories since Git must search the entire worktree to enumerate them. When it is too slow, Git prints advice with the elapsed search time and a suggestion to disable the search using the -uno option. This suggestion also carries a warning that might scare off some users.

However, these days, -uno isn't the only option. Git can reduce the size and time of the untracked file search when the core.untrackedCache and core.fsmonitor features are enabled by caching results from previous git status invocations.

Therefore, update the git status man page to explain the various configuration options, and update the advice to provide more detail about the current configuration and to refer to the updated documentation.

---
status: modernize git-status "slow untracked files" advice

Here is version 8 for this patch.

Changes since v7:

* Moved tests from new test script to existing one, in order not to needlessly waste a test script number for such a small feature. Two caveats:
* The use of test_config in a subshell result in: 'error: bug in the test script: test_when_finished does nothing in a subshell', so I've had to resort to using plain old git config instead.
* A test higher in the script is doing git config --global advice.statusuoption false, so I now have to set it locally in my isolated test repo.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1384%2Frudyrigot%2Fadvice_statusFsmonitor-v8
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1384/rudyrigot/advice_statusFsmonitor-v8
Pull-Request: https://github.com/gitgitgadget/git/pull/1384

Range-diff vs v7:

1: 871a9becbdf ! 1: 16e3721515b status: modernize git-status "slow untracked files" advice

     @@ Documentation/git-status.txt: during the write may conflict with other simultane
       --------
       linkgit:gitignore[5]
      
     - ## t/t7065-wtstatus-slow.sh (new) ##
     -@@
     -+#!/bin/sh
     -+
     -+test_description='test status when slow untracked files'
     -+
     -+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
     -+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
     -+
     -+GIT_TEST_UF_DELAY_WARNING=1
     -+export GIT_TEST_UF_DELAY_WARNING
     -+
     -+. ./test-lib.sh
     -+
     -+test_expect_success setup '
     -+	cat >.gitignore <<-\EOF &&
     -+	/actual
     -+	/expected
     -+	/out
     -+	EOF
     -+	git add .gitignore &&
     -+	git commit -m "Add .gitignore"
     + ## t/t7508-status.sh ##
     +@@ t/t7508-status.sh: test_expect_success 'racy timestamps will be fixed for dirty worktree' '
     + 	! test_is_magic_mtime .git/index
     + '
     + 
     ++test_expect_success 'setup slow status advice' '
     ++	GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main git init slowstatus &&
     ++	(
     ++		cd slowstatus &&
     ++		cat >.gitignore <<-\EOF &&
     ++		/actual
     ++		/expected
     ++		/out
     ++		EOF
     ++		git add .gitignore &&
     ++		git commit -m "Add .gitignore" &&
     ++		git config advice.statusuoption true
     ++	)
      +'
      +
     -+test_expect_success 'when core.untrackedCache and fsmonitor are unset' '
     -+	test_unconfig core.untrackedCache &&
     -+	test_unconfig core.fsmonitor &&
     -+	git status >out &&
     -+	sed "s/[0-9]\.[0-9][0-9]/X/g" out >actual &&
     -+	cat >expected <<-\EOF &&
     -+	On branch main
     -+
     -+	It took X seconds to enumerate untracked files.
     -+	See '"'"'git help status'"'"' for information on how to improve this.
     -+
     -+	nothing to commit, working tree clean
     -+	EOF
     -+	test_cmp expected actual
     ++test_expect_success 'slow status advice when core.untrackedCache and fsmonitor are unset' '
     ++	(
     ++		cd slowstatus &&
     ++		git config core.untrackedCache false &&
     ++		git config core.fsmonitor false &&
     ++		GIT_TEST_UF_DELAY_WARNING=1 git status >out &&
     ++		sed "s/[0-9]\.[0-9][0-9]/X/g" out >actual &&
     ++		cat >expected <<-\EOF &&
     ++		On branch main
     ++
     ++		It took X seconds to enumerate untracked files.
     ++		See '"'"'git help status'"'"' for information on how to improve this.
     ++
     ++		nothing to commit, working tree clean
     ++		EOF
     ++		test_cmp expected actual
     ++	)
      +'
      +
     -+test_expect_success 'when core.untrackedCache true, but not fsmonitor' '
     -+	test_config core.untrackedCache true &&
     -+	test_unconfig core.fsmonitor &&
     -+	git status >out &&
     -+	sed "s/[0-9]\.[0-9][0-9]/X/g" out >actual &&
     -+	cat >expected <<-\EOF &&
     -+	On branch main
     -+
     -+	It took X seconds to enumerate untracked files.
     -+	See '"'"'git help status'"'"' for information on how to improve this.
     -+
     -+	nothing to commit, working tree clean
     -+	EOF
     -+	test_cmp expected actual
     ++test_expect_success 'slow status advice when core.untrackedCache true, but not fsmonitor' '
     ++	(
     ++		cd slowstatus &&
     ++		git config core.untrackedCache true &&
     ++		git config core.fsmonitor false &&
     ++		GIT_TEST_UF_DELAY_WARNING=1 git status >out &&
     ++		sed "s/[0-9]\.[0-9][0-9]/X/g" out >actual &&
     ++		cat >expected <<-\EOF &&
     ++		On branch main
     ++
     ++		It took X seconds to enumerate untracked files.
     ++		See '"'"'git help status'"'"' for information on how to improve this.
     ++
     ++		nothing to commit, working tree clean
     ++		EOF
     ++		test_cmp expected actual
     ++	)
      +'
      +
     -+test_expect_success 'when core.untrackedCache true, and fsmonitor' '
     -+	test_config core.untrackedCache true &&
     -+	test_config core.fsmonitor true &&
     -+	git status >out &&
     -+	sed "s/[0-9]\.[0-9][0-9]/X/g" out >actual &&
     -+	cat >expected <<-\EOF &&
     -+	On branch main
     -+
     -+	It took X seconds to enumerate untracked files,
     -+	but the results were cached, and subsequent runs may be faster.
     -+	See '"'"'git help status'"'"' for information on how to improve this.
     -+
     -+	nothing to commit, working tree clean
     -+	EOF
     -+	test_cmp expected actual
     ++test_expect_success 'slow status advice when core.untrackedCache true, and fsmonitor' '
     ++	(
     ++		cd slowstatus &&
     ++		git config core.untrackedCache true &&
     ++		git config core.fsmonitor true &&
     ++		GIT_TEST_UF_DELAY_WARNING=1 git status >out &&
     ++		sed "s/[0-9]\.[0-9][0-9]/X/g" out >actual &&
     ++		cat >expected <<-\EOF &&
     ++		On branch main
     ++
     ++		It took X seconds to enumerate untracked files,
     ++		but the results were cached, and subsequent runs may be faster.
     ++		See '"'"'git help status'"'"' for information on how to improve this.
     ++
     ++		nothing to commit, working tree clean
     ++		EOF
     ++		test_cmp expected actual
     ++	)
      +'
      +
     -+test_done
     + test_done
      
       ## wt-status.c ##
      @@

 Documentation/git-status.txt | 59 +++++++++++++++++++++++++++++
 t/t7508-status.sh            | 73 ++++++++++++++++++++++++++++++++++++
 wt-status.c                  | 28 +++++++++++---
 3 files changed, 155 insertions(+), 5 deletions(-)

index 5e438a7fdc1..570c36e07c1 100644
@@ -457,6 +457,65 @@ during the write may conflict with other simultaneous processes, causing
 them to fail. Scripts running `status` in the background should consider
 using `git --no-optional-locks status` (see linkgit:git[1] for details).
 
+UNTRACKED FILES AND STATUS SPEED
+--------------------------------
+
+`git status` can be very slow in large worktrees if/when it
+needs to search for untracked files and directories. There are
+many configuration options available to speed this up by either
+avoiding the work or making use of cached results from previous
+Git commands. There is no single optimum set of settings right
+for everyone.  Here is a brief summary of the relevant options
+to help you choose which is right for you.
+
+* First, you may want to run `git status` again. Your current
+	configuration may already be caching `git status` results,
+	so it could be faster on subsequent runs.
+
+* The `--untracked-files=no` flag or the
+	`status.showUntrackedfiles=false` config (see above for both) :
+	indicate that `git status` should not report untracked
+	files. This is the fastest option. `git status` will not list
+	the untracked files, so you need to be careful to remember if
+	you create any new files and manually `git add` them.
+
+* `advice.statusUoption=false` (see linkgit:git-config[1]) :
+	this config option disables a warning message when the search
+	for untracked files takes longer than desired. In some large
+	repositories, this message may appear frequently and not be a
+	helpful signal.
+
+* `core.untrackedCache=true` (see linkgit:git-update-index[1]) :
+	enable the untracked cache feature and only search directories
+	that have been modified since the previous `git status` command.
+	Git remembers the set of untracked files within each directory
+	and assumes that if a directory has not been modified, then
+	the set of untracked files within has not changed.  This is much
+	faster than enumerating the contents of every directory, but still
+	not without cost, because Git still has to search for the set of
+	modified directories. The untracked cache is stored in the
+	`.git/index` file. The reduced cost of searching for untracked
+	files is offset slightly by the increased size of the index and
+	the cost of keeping it up-to-date. That reduced search time is
+	usually worth the additional size.
+
+* `core.untrackedCache=true` and `core.fsmonitor=true` or
+	`core.fsmonitor=<hook_command_pathname>` (see
+	linkgit:git-update-index[1]) : enable both the untracked cache
+	and FSMonitor features and only search directories that have
+	been modified since the previous `git status` command.  This
+	is faster than using just the untracked cache alone because
+	Git can also avoid searching for modified directories.  Git
+	only has to enumerate the exact set of directories that have
+	changed recently. While the FSMonitor feature can be enabled
+	without the untracked cache, the benefits are greatly reduced
+	in that case.
+
+Note that after you turn on the untracked cache and/or FSMonitor
+features it may take a few `git status` commands for the various
+caches to warm up before you see improved command times.  This is
+normal.
+
 SEE ALSO
 --------
 linkgit:gitignore[5]

index 5813174896c..1f6d64e759f 100644
@@ -18,8 +18,10 @@
 #include "worktree.h"
 #include "lockfile.h"
 #include "sequencer.h"
+#include "fsmonitor-settings.h"
 
 #define AB_DELAY_WARNING_IN_MS (2 * 1000)
+#define UF_DELAY_WARNING_IN_MS (2 * 1000)
 
 static const char cut_line[] =
 "------------------------ >8 ------------------------\n";

@@ -1205,6 +1207,13 @@ static void wt_longstatus_print_tracking(struct wt_status *s)
 	strbuf_release(&sb);
 }
 
+static int uf_was_slow(uint32_t untracked_in_ms)
+{
+	if (getenv("GIT_TEST_UF_DELAY_WARNING"))
+		untracked_in_ms += UF_DELAY_WARNING_IN_MS + 1;
+	return UF_DELAY_WARNING_IN_MS < untracked_in_ms;
+}
+
 static void show_merge_in_progress(struct wt_status *s,
 				   const char *color)
 {

@@ -1814,6 +1823,7 @@ static void wt_longstatus_print(struct wt_status *s)
 {
 	const char *branch_color = color(WT_STATUS_ONBRANCH, s);
 	const char *branch_status_color = color(WT_STATUS_HEADER, s);
+	enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(s->repo);
 
 	if (s->branch) {
 		const char *on_what = _("On branch ");

@@ -1870,13 +1880,21 @@ static void wt_longstatus_print(struct wt_status *s)
 		wt_longstatus_print_other(s, &s->untracked, _("Untracked files"), "add");
 		if (s->show_ignored_mode)
 			wt_longstatus_print_other(s, &s->ignored, _("Ignored files"), "add -f");
-		if (advice_enabled(ADVICE_STATUS_U_OPTION) && 2000 < s->untracked_in_ms) {
+		if (advice_enabled(ADVICE_STATUS_U_OPTION) && uf_was_slow(s->untracked_in_ms)) {
 			status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
+			if (fsm_mode > FSMONITOR_MODE_DISABLED) {
+				status_printf_ln(s, GIT_COLOR_NORMAL,
+						_("It took %.2f seconds to enumerate untracked files,\n"
+						"but the results were cached, and subsequent runs may be faster."),
+						s->untracked_in_ms / 1000.0);
+			} else {
+				status_printf_ln(s, GIT_COLOR_NORMAL,
+						_("It took %.2f seconds to enumerate untracked files."),
+						s->untracked_in_ms / 1000.0);
+			}
 			status_printf_ln(s, GIT_COLOR_NORMAL,
-					 _("It took %.2f seconds to enumerate untracked files. 'status -uno'\n"
-					   "may speed it up, but you have to be careful not to forget to add\n"
-					   "new files yourself (see 'git help status')."),
-					 s->untracked_in_ms / 1000.0);
+					_("See 'git help status' for information on how to improve this."));
+			status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
 		}
 	} else if (s->committable)
 		status_printf_ln(s, GIT_COLOR_NORMAL, _("Untracked files not listed%s"),

base-commit: 319605f8f00e402f3ea758a02c63534ff800a711

gitgitgadget

"time" I can sort of understand ("can reduce the time taken to enumerate untracked files" is how I may phrase it, though), but what did you want to say with "size"?

Lose "Therefore, "; the resulting text would be much easier to follow.

"STATUS SPEED" somehow does not sound quite grammatical. Perhaps "untracked files and performance" or something instead?

	"It took 92.34 seconds to enumerate..."

	static int uf_was_slow(struct wt_status *s)
	{
		if (getenv("GIT_TEST_UF_DETAIL_WARNING"))
			s->untracked_in_ms = 3.25;
		return UF_DELAY_WARNING_IN_MS < s->untracked_in_ms;
	}

	GIT_TEST_UF_DETAIL_WARNING=1 git status >actual &&
	cat >expect <<-\EOF &&
	...
	It took 3.25 seconds to enumerate ...
	EOF
	test_cmp expect actual

Thanks for the amazing feedback, and sorry for the delay.

This bit was provided by a past reviewer so I hope I don't misrepresent it, but I think the idea was to convey that the reason it's faster is because the part of the codebase it will do active work on is smaller. I don't think it provides compellingly more information for end users, compared to just mentioning time, so I'll simplify with your proposal here.

I agree it would be an improvement, I'm going to try to do this. This doesn't feel like a much more involved change compared to your alternative suggestion of setting a hardcoded test value for s->untracked_in_ms, so it feels like there's not much to lose from doing it this way, while users would gain some nice configurability.

For transparency, my intuition is that I'm not sure there will be use cases where the config will be meaningfully leveraged by users. My gut is that the current cut-off time is an arbitrary UX perception cut-off, so I'm not sure it would need to be different depending on given repo situations. But I'm also very aware that I could be wrong, and this could open use cases that I'm not thinking about. And to your point, it would be sensible to use it as a test input anyway, so we might as well make it a user-facing tweak; so, I'm on board.

Yup, you guessed right. I'll be sure to change the regular expression to be resilient to double-digit times.

Indeed, it's not useful to anything, and shouldn't be there, I'll fix this too.

All of the other comments are crystal clear, and I intend to implement them exactly as advised. You can expect a new patch this week, maybe even today.

Thanks a lot again!

Alright, I tried the "status.enumerateUntrackedDelayMS" approach, but I couldn't pull it off and now I am stumped.

This is somewhat frustrating, so I'd welcome guidance if anyone has time and is interested. Since this doesn't actually work, I don't think I should create an actual patch for it on the mailing list, so here are two other ways to show what I've got, I hope they're acceptable:

- in Gist form:
https://gist.github.com/rudyrigot/aa3e8e5ddb4f71fdc7fc0e92d9b7a4b8
- in GitHub compare form:
https://github.com/git/git/compare/master...rudyrigot:git:status_enumerateUntrackedDelayMS

The issues I'm seeing:

- No matter how I set the config from the test, it doesn't seem to have any effect. I'm thinking I might be doing something wrong in how I set the value, which I've done in git_status_config in builtin/commit.c, which very well may be the wrong place.
- Therefore, I've been testing things by changing the default value in wt_status_prepare in wt-status.c. Setting it at 0 and making the operator <= instead of < makes the advice display, which tells me that the logic is sound. Setting at its intended value of 2000 doesn't display the advice message, as expected. But setting it at -1 also doesn't display it. I'm a bit puzzled about why that would be, and I'm
wondering: maybe the int is unsigned? It doesn't look like it based on how the structure field is declared in wt-status.h, but I know my own limits in C so I could be wrong.

Now, I'm also well aware that Junio raised that advice leaving the door wide open to not actually solve this as part of this patch; and I did express in my previous reply that I am not intuitively convinced there is much value to it for users, although I could be wrong of course. So with that, if it's better to let it go, that is fine by me too.

With that in mind, I implemented the alternative that Junio was proposing instead (assigning the value of s->untracked_in_ms), and it seems to work all good. It just passed CI, so I'm about to submit that as a patch, with every other piece of feedback also addressed.

Unrelated note: I noticed that the first 2 bits of feedback applied to docs that were part of past patches, but were removed in the last patch. The rest of the doc feedback was current, so I was able to implement them, but obviously I couldn't implement the first 2 ones, since the issues they're about are gone.

From: Rudy Rigot <[email protected]>

git status can be slow when there are a large number of untracked files and directories since Git must search the entire worktree to enumerate them. When it is too slow, Git prints advice with the elapsed search time and a suggestion to disable the search using the -uno option. This suggestion also carries a warning that might scare off some users.

However, these days, -uno isn't the only option. Git can reduce the size and time of the untracked file search when the core.untrackedCache and core.fsmonitor features are enabled by caching results from previous git status invocations.

Therefore, update the git status man page to explain the various configuration options, and update the advice to provide more detail about the current configuration and to refer to the updated documentation.

---
status: modernize git-status "slow untracked files" advice

Here is version 9 for this patch.

Changes since v8:

* Improved tests.
* The untracked files delay measured is now set to always the same value in test cases. That has allowed to remove all sed calls from tests.
* Improved documentation.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1384%2Frudyrigot%2Fadvice_statusFsmonitor-v9
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1384/rudyrigot/advice_statusFsmonitor-v9
Pull-Request: https://github.com/gitgitgadget/git/pull/1384

Range-diff vs v8:

1: 16e3721515b ! 1: fcb298e6e5a status: modernize git-status "slow untracked files" advice
@@ Documentation/git-status.txt: during the write may conflict with other simultane
them to fail. Scripts running status in the background should consider using git --no-optional-locks status (see linkgit:git[1] for details).

-+UNTRACKED FILES AND STATUS SPEED
-+--------------------------------
++UNTRACKED FILES AND PERFORMANCE
++-------------------------------
+
+git status can be very slow in large worktrees if/when it
+needs to search for untracked files and directories. There are +many configuration options available to speed this up by either +avoiding the work or making use of cached results from previous +Git commands. There is no single optimum set of settings right
-+for everyone. Here is a brief summary of the relevant options
-+to help you choose which is right for you.
-+
-+* First, you may want to run git status again. Your current
-+ configuration may already be caching git status results,
-+ so it could be faster on subsequent runs.
++for everyone. We'll list a summary of the relevant options to help ++you, but before going into the list, you may want to run git status ++again, because your configuration may already be caching git status ++results, so it could be faster on subsequent runs.
+
+* The --untracked-files=no flag or the
-+ status.showUntrackedfiles=false config (see above for both) :
++ status.showUntrackedfiles=false config (see above for both):
+ indicate that git status should not report untracked
+ files. This is the fastest option. git status will not list
+ the untracked files, so you need to be careful to remember if
+ you create any new files and manually git add them.
+
-+* advice.statusUoption=false (see linkgit:git-config[1]) :
-+ this config option disables a warning message when the search
-+ for untracked files takes longer than desired. In some large
-+ repositories, this message may appear frequently and not be a
-+ helpful signal.
++* advice.statusUoption=false (see linkgit:git-config[1]):
++ setting this variable to false disables the warning message
++ given when enumerating untracked files takes more than 2
++ seconds. In a large project, it may take longer and the user
++ may have already accepted the trade off (e.g. using "-uno" may
++ not be an acceptable option for the user), in which case, there
++ is no point issuing the warning message, and in such a case,
++ disabling the warning may be the best.
+
-+* core.untrackedCache=true (see linkgit:git-update-index[1]) :
++* core.untrackedCache=true (see linkgit:git-update-index[1]):
+ enable the untracked cache feature and only search directories
+ that have been modified since the previous git status command.
+ Git remembers the set of untracked files within each directory
@@ Documentation/git-status.txt: during the write may conflict with other simultane
+
+* core.untrackedCache=true and core.fsmonitor=true or
+ core.fsmonitor=<hook_command_pathname> (see
-+ linkgit:git-update-index[1]) : enable both the untracked cache
++ linkgit:git-update-index[1]): enable both the untracked cache
+ and FSMonitor features and only search directories that have
+ been modified since the previous git status command. This
+ is faster than using just the untracked cache alone because
@@ t/t7508-status.sh: test_expect_success 'racy timestamps will be fixed for dirty
+ cd slowstatus &&
+ git config core.untrackedCache false &&
+ git config core.fsmonitor false &&
-+ GIT_TEST_UF_DELAY_WARNING=1 git status >out &&
-+ sed "s/[0-9]\.[0-9][0-9]/X/g" out >actual &&
++ GIT_TEST_UF_DELAY_WARNING=1 git status >actual &&
+ cat >expected <<-\EOF &&
+ On branch main
+
-+ It took X seconds to enumerate untracked files.
-+ See '"'"'git help status'"'"' for information on how to improve this.
++ It took 3.25 seconds to enumerate untracked files.
++ See '\''git help status'\'' for information on how to improve this.
+
+ nothing to commit, working tree clean
+ EOF
@@ t/t7508-status.sh: test_expect_success 'racy timestamps will be fixed for dirty
+ cd slowstatus &&
+ git config core.untrackedCache true &&
+ git config core.fsmonitor false &&
-+ GIT_TEST_UF_DELAY_WARNING=1 git status >out &&
-+ sed "s/[0-9]\.[0-9][0-9]/X/g" out >actual &&
++ GIT_TEST_UF_DELAY_WARNING=1 git status >actual &&
+ cat >expected <<-\EOF &&
+ On branch main
+
-+ It took X seconds to enumerate untracked files.
-+ See '"'"'git help status'"'"' for information on how to improve this.
++ It took 3.25 seconds to enumerate untracked files.
++ See '\''git help status'\'' for information on how to improve this.
+
+ nothing to commit, working tree clean
+ EOF
@@ t/t7508-status.sh: test_expect_success 'racy timestamps will be fixed for dirty
+ cd slowstatus &&
+ git config core.untrackedCache true &&
+ git config core.fsmonitor true &&
-+ GIT_TEST_UF_DELAY_WARNING=1 git status >out &&
-+ sed "s/[0-9]\.[0-9][0-9]/X/g" out >actual &&
++ GIT_TEST_UF_DELAY_WARNING=1 git status >actual &&
+ cat >expected <<-\EOF &&
+ On branch main
+
-+ It took X seconds to enumerate untracked files,
++ It took 3.25 seconds to enumerate untracked files,
+ but the results were cached, and subsequent runs may be faster.
-+ See '"'"'git help status'"'"' for information on how to improve this.
++ See '\''git help status'\'' for information on how to improve this.
+
+ nothing to commit, working tree clean
+ EOF
@@ wt-status.c: static void wt_longstatus_print_tracking(struct wt_status *s)
strbuf_release(&sb);
}

-+static int uf_was_slow(uint32_t untracked_in_ms)
++static int uf_was_slow(struct wt_status *s)
+{
+ if (getenv("GIT_TEST_UF_DELAY_WARNING"))
-+ untracked_in_ms += UF_DELAY_WARNING_IN_MS + 1;
-+ return UF_DELAY_WARNING_IN_MS < untracked_in_ms;
++ s->untracked_in_ms = 3250;
++ return UF_DELAY_WARNING_IN_MS < s->untracked_in_ms;
+}
+
static void show_merge_in_progress(struct wt_status *s,
@@ wt-status.c: static void wt_longstatus_print(struct wt_status *s)
if (s->show_ignored_mode)
wt_longstatus_print_other(s, &s->ignored, _("Ignored files"), "add -f");
- if (advice_enabled(ADVICE_STATUS_U_OPTION) && 2000 < s->untracked_in_ms) {
-+ if (advice_enabled(ADVICE_STATUS_U_OPTION) && uf_was_slow(s->untracked_in_ms)) {
++ if (advice_enabled(ADVICE_STATUS_U_OPTION) && uf_was_slow(s)) {
status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
+ if (fsm_mode > FSMONITOR_MODE_DISABLED) {
+ status_printf_ln(s, GIT_COLOR_NORMAL,

 Documentation/git-status.txt | 60 +++++++++++++++++++++++++++++++
 t/t7508-status.sh            | 70 ++++++++++++++++++++++++++++++++++++
 wt-status.c                  | 28 ++++++++++++---
 3 files changed, 153 insertions(+), 5 deletions(-)

index 5e438a7fdc1..a051b1e8f38 100644
@@ -457,6 +457,66 @@ during the write may conflict with other simultaneous processes, causing
 them to fail. Scripts running `status` in the background should consider
 using `git --no-optional-locks status` (see linkgit:git[1] for details).
 
+UNTRACKED FILES AND PERFORMANCE
+-------------------------------
+
+`git status` can be very slow in large worktrees if/when it
+needs to search for untracked files and directories. There are
+many configuration options available to speed this up by either
+avoiding the work or making use of cached results from previous
+Git commands. There is no single optimum set of settings right
+for everyone. We'll list a summary of the relevant options to help
+you, but before going into the list, you may want to run `git status`
+again, because your configuration may already be caching `git status`
+results, so it could be faster on subsequent runs.
+
+* The `--untracked-files=no` flag or the
+	`status.showUntrackedfiles=false` config (see above for both):
+	indicate that `git status` should not report untracked
+	files. This is the fastest option. `git status` will not list
+	the untracked files, so you need to be careful to remember if
+	you create any new files and manually `git add` them.
+
+* `advice.statusUoption=false` (see linkgit:git-config[1]):
+	setting this variable to `false` disables the warning message
+	given when enumerating untracked files takes more than 2
+	seconds.  In a large project, it may take longer and the user
+	may have already accepted the trade off (e.g. using "-uno" may
+	not be an acceptable option for the user), in which case, there
+	is no point issuing the warning message, and in such a case,
+	disabling the warning may be the best.
+
+* `core.untrackedCache=true` (see linkgit:git-update-index[1]):
+	enable the untracked cache feature and only search directories
+	that have been modified since the previous `git status` command.
+	Git remembers the set of untracked files within each directory
+	and assumes that if a directory has not been modified, then
+	the set of untracked files within has not changed.  This is much
+	faster than enumerating the contents of every directory, but still
+	not without cost, because Git still has to search for the set of
+	modified directories. The untracked cache is stored in the
+	`.git/index` file. The reduced cost of searching for untracked
+	files is offset slightly by the increased size of the index and
+	the cost of keeping it up-to-date. That reduced search time is
+	usually worth the additional size.
+
+* `core.untrackedCache=true` and `core.fsmonitor=true` or
+	`core.fsmonitor=<hook_command_pathname>` (see
+	linkgit:git-update-index[1]): enable both the untracked cache
+	and FSMonitor features and only search directories that have
+	been modified since the previous `git status` command.  This
+	is faster than using just the untracked cache alone because
+	Git can also avoid searching for modified directories.  Git
+	only has to enumerate the exact set of directories that have
+	changed recently. While the FSMonitor feature can be enabled
+	without the untracked cache, the benefits are greatly reduced
+	in that case.
+
+Note that after you turn on the untracked cache and/or FSMonitor
+features it may take a few `git status` commands for the various
+caches to warm up before you see improved command times.  This is
+normal.
+
 SEE ALSO
 --------
 linkgit:gitignore[5]

index 5813174896c..b430d25da43 100644
@@ -18,8 +18,10 @@
 #include "worktree.h"
 #include "lockfile.h"
 #include "sequencer.h"
+#include "fsmonitor-settings.h"
 
 #define AB_DELAY_WARNING_IN_MS (2 * 1000)
+#define UF_DELAY_WARNING_IN_MS (2 * 1000)
 
 static const char cut_line[] =
 "------------------------ >8 ------------------------\n";

@@ -1205,6 +1207,13 @@ static void wt_longstatus_print_tracking(struct wt_status *s)
 	strbuf_release(&sb);
 }
 
+static int uf_was_slow(struct wt_status *s)
+{
+	if (getenv("GIT_TEST_UF_DELAY_WARNING"))
+		s->untracked_in_ms = 3250;
+	return UF_DELAY_WARNING_IN_MS < s->untracked_in_ms;
+}
+
 static void show_merge_in_progress(struct wt_status *s,
 				   const char *color)
 {

@@ -1814,6 +1823,7 @@ static void wt_longstatus_print(struct wt_status *s)
 {
 	const char *branch_color = color(WT_STATUS_ONBRANCH, s);
 	const char *branch_status_color = color(WT_STATUS_HEADER, s);
+	enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(s->repo);
 
 	if (s->branch) {
 		const char *on_what = _("On branch ");

@@ -1870,13 +1880,21 @@ static void wt_longstatus_print(struct wt_status *s)
 		wt_longstatus_print_other(s, &s->untracked, _("Untracked files"), "add");
 		if (s->show_ignored_mode)
 			wt_longstatus_print_other(s, &s->ignored, _("Ignored files"), "add -f");
-		if (advice_enabled(ADVICE_STATUS_U_OPTION) && 2000 < s->untracked_in_ms) {
+		if (advice_enabled(ADVICE_STATUS_U_OPTION) && uf_was_slow(s)) {
 			status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
+			if (fsm_mode > FSMONITOR_MODE_DISABLED) {
+				status_printf_ln(s, GIT_COLOR_NORMAL,
+						_("It took %.2f seconds to enumerate untracked files,\n"
+						"but the results were cached, and subsequent runs may be faster."),
+						s->untracked_in_ms / 1000.0);
+			} else {
+				status_printf_ln(s, GIT_COLOR_NORMAL,
+						_("It took %.2f seconds to enumerate untracked files."),
+						s->untracked_in_ms / 1000.0);
+			}
 			status_printf_ln(s, GIT_COLOR_NORMAL,
-					 _("It took %.2f seconds to enumerate untracked files. 'status -uno'\n"
-					   "may speed it up, but you have to be careful not to forget to add\n"
-					   "new files yourself (see 'git help status')."),
-					 s->untracked_in_ms / 1000.0);
+					_("See 'git help status' for information on how to improve this."));
+			status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
 		}
 	} else if (s->committable)
 		status_printf_ln(s, GIT_COLOR_NORMAL, _("Untracked files not listed%s"),

base-commit: 319605f8f00e402f3ea758a02c63534ff800a711

gitgitgadget

Looking pretty good (I thought you were going to update the proposed log message, too, though). Let's replace and merge it down to 'next' to cook during the pre-release freeze period.

Thanks.

Oh, I'm sorry, what do you mean by updating the proposed log message? I'm looking back at past feedback, and I can't seem to match it to an unaddressed one. It was not intentional, so I would be very glad to fix it if you'd like, I'll gladly leave it up to you.

Thanks a lot!

I was referring to this part:

in

https://lore.kernel.org/git/CANaDLW+ukK2GU7NzkCvXVNc9DX3_93Pp+PHq-WcLpRJizPidVA@mail.gmail.com/

you sent earlier.

No big deal, and thanks for working on this.

Oh, now I understand!

But I had skipped it because I couldn't find it in my patch, and even now, I'm afraid I can't. It looks to me like part of an old patch, a paragraph that was rewritten 2 patches back. Is it possible that you found it in the lines that were being removed on the patch that you commented on, for instance?

If I'm correct, then yeay! No problem in the end, this won't reach users. If I'm looking wrong though, then I'm very sorry about the miss. I also do get how this may not be worth a new round just for this fix.

Thanks a lot for your (and everybody else's) guidance through this. This was my first patch, and I'm delighted about the solid shape it's landing with; I'm hoping it is the first of many.

Thanks a lot!

Sorry, I should have thought of this when making suggestions in my earlier reviews. For completeness, in case you run across this sort of issue again if submitting patches in the future, it is possible to get this working by using the -C option (change to directory) with test_config() outside of the subshell. For instance:

    test_expect_sucess 'some title' '
        test_config -C slowstatus core.untrackedCache false &&
        test_config -C slowstatus core.fsmonitor false &&
        (
            cd slowstatus &&
            GIT_TEST_UF_DELAY_WARNING=1 git status >out &&
            ...
        )
    '