[PATCH] fsmonitor: long status advice adapted to the fsmonitor use case

[PATCH] fsmonitor: long status advice adapted to the fsmonitor use case

[Rudy Rigot via GitGitGadget]

From: Rudy Rigot <rudy.rigot@gmail.com>

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.

Signed-off-by: Rudy Rigot <rudy.rigot@gmail.com>
---
    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(-)

diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt
index a00d0100a82..e8ebcf1b023 100644
--- a/Documentation/config/advice.txt
+++ b/Documentation/config/advice.txt
@@ -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
diff --git a/advice.c b/advice.c
index fd189689437..448b11ef273 100644
--- a/advice.c
+++ b/advice.c
@@ -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 },
diff --git a/advice.h b/advice.h
index 07e0f76833e..a458619a160 100644
--- a/advice.h
+++ b/advice.h
@@ -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,
diff --git a/wt-status.c b/wt-status.c
index 5813174896c..d6c7f0fa21a 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -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

Re: [PATCH] fsmonitor: long status advice adapted to the fsmonitor use case

[Rudy Rigot]

> I considered just suppressing that piece of advice entirely for
repositories with fsmonitor disabled

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

Re: [PATCH] fsmonitor: long status advice adapted to the fsmonitor use case

[Jeff Hostetler]

On 10/15/22 9:04 AM, Rudy Rigot via GitGitGadget wrote:
> From: Rudy Rigot <rudy.rigot@gmail.com>
> 
> 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.

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 ?


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


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)

> 
> 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.
[...]

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

Re: [PATCH] fsmonitor: long status advice adapted to the fsmonitor use case

[Rudy Rigot]

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

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.


> WRT testing

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.

Re: [PATCH] fsmonitor: long status advice adapted to the fsmonitor use case

[Jeff Hostetler]

On 10/17/22 12:59 PM, Rudy Rigot wrote:
>> 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.
> 
> 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.

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.

> 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')."

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

Re: [PATCH] fsmonitor: long status advice adapted to the fsmonitor use case

[Rudy Rigot]

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:

> Your git status run was slow, here are some ways to optimize it.
> https://git-scm.com/docs/git-status#_performance_optimizations

(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:

> Your git status run was slow, here are some ways to optimize it.
> https://git-scm.com/docs/git-status#_performance_optimizations
>
> Your git status run was cached.

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:

> Your git status run was slow, here are some ways to optimize it.
> https://git-scm.com/docs/git-status#_performance_optimizations
>
> Your git status run was cached, it may be faster on your next runs.

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!

Re: [PATCH] fsmonitor: long status advice adapted to the fsmonitor use case

[Jeff Hostetler]

On 10/20/22 4:17 PM, Rudy Rigot wrote:
> 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.
 > [...]

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

Jeff

[PATCH v2] status: long status advice adapted to recent capabilities

[Rudy Rigot via GitGitGadget]

From: Rudy Rigot <rudy.rigot@gmail.com>

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.

Signed-off-by: Rudy Rigot <rudy.rigot@gmail.com>
---
    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 <rudy.rigot@gmail.com>
      
       ## 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.
      
          Signed-off-by: Rudy Rigot <rudy.rigot@gmail.com>
      
     - ## 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

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 54a4b29b473..3d92e5fd018 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -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]
diff --git a/t/t7065-wtstatus-slow.sh b/t/t7065-wtstatus-slow.sh
new file mode 100755
index 00000000000..92c053eaa64
--- /dev/null
+++ b/t/t7065-wtstatus-slow.sh
@@ -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
diff --git a/t/t7065/no_untrackedcache_no_fsmonitor b/t/t7065/no_untrackedcache_no_fsmonitor
new file mode 100644
index 00000000000..e346deaa1db
--- /dev/null
+++ b/t/t7065/no_untrackedcache_no_fsmonitor
@@ -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)
diff --git a/t/t7065/with_untrackedcache_no_fsmonitor b/t/t7065/with_untrackedcache_no_fsmonitor
new file mode 100644
index 00000000000..a649d367493
--- /dev/null
+++ b/t/t7065/with_untrackedcache_no_fsmonitor
@@ -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)
diff --git a/t/t7065/with_untrackedcache_with_fsmonitor b/t/t7065/with_untrackedcache_with_fsmonitor
new file mode 100644
index 00000000000..d5e95d984f8
--- /dev/null
+++ b/t/t7065/with_untrackedcache_with_fsmonitor
@@ -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)
diff --git a/wt-status.c b/wt-status.c
index 5813174896c..be903c6e294 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -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

Re: [PATCH v2] status: long status advice adapted to recent capabilities

[Jeff Hostetler]

On 10/28/22 8:06 PM, Rudy Rigot via GitGitGadget wrote:
> From: Rudy Rigot <rudy.rigot@gmail.com>
> 
> 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.

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


[...]
> diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
> index 54a4b29b473..3d92e5fd018 100644
> --- a/Documentation/git-status.txt
> +++ b/Documentation/git-status.txt
> @@ -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`.
> +

I hate to suggest a complete rewrite as I myself struggle with
how to phrase things all toooooo often, but let me offer another
starting point and see what you think:

     `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.

Something like that.  Hope this helps.  (And again, sorry for the
rewrite.)


[...]
> diff --git a/t/t7065-wtstatus-slow.sh b/t/t7065-wtstatus-slow.sh
> new file mode 100755
> index 00000000000..92c053eaa64
> --- /dev/null
> +++ b/t/t7065-wtstatus-slow.sh
> @@ -0,0 +1,40 @@
> +#!/bin/sh
[...]
> +
> +test_done
> \ No newline at end of file

small nit: we should have a final LF at the end of the file.

I'm going to skip over the test cases because I'm running
short on time this afternoon.


[...]
> diff --git a/wt-status.c b/wt-status.c
[...]
>   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", "");
> +			}

I'm not sure I like the various mixture of messages here.  Maybe
it would be better with a single simple message:

     _("It took %.2f seconds to enumerate untracked files.\n"
       "See 'git help status' for information on how to improve this.")

This keeps all of the information in the documentation rather
than having part of it here in the code.

Also, we should refer to the documentation via `git help` rather
than as a link to the website.


Thanks,
Jeff

Re: [PATCH v2] status: long status advice adapted to recent capabilities

[Rudy Rigot]

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.


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

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.


> I hate to suggest a complete rewrite
> [...]
> Something like that.  Hope this helps.  (And again, sorry for the
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!


> small nit: we should have a final LF at the end of the file.

Sounds good, will fix.


> I'm going to skip over the test cases because I'm running
> short on time this afternoon.

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.


> Also, we should refer to the documentation via `git help` rather
> than as a link to the website.

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


> I'm not sure I like the various mixture of messages here.  Maybe
> it would be better with a single simple message:

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.

[PATCH v3] status: long status advice adapted to recent capabilities

[Rudy Rigot via GitGitGadget]

From: Rudy Rigot <rudy.rigot@gmail.com>

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.

Signed-off-by: Rudy Rigot <rudy.rigot@gmail.com>
---
    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.
      
          Signed-off-by: Rudy Rigot <rudy.rigot@gmail.com>
      
     @@ 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

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 54a4b29b473..95f4ed95e96 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -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]
diff --git a/t/t7065-wtstatus-slow.sh b/t/t7065-wtstatus-slow.sh
new file mode 100755
index 00000000000..23c37ea71e7
--- /dev/null
+++ b/t/t7065-wtstatus-slow.sh
@@ -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
diff --git a/t/t7065/no_untrackedcache_no_fsmonitor b/t/t7065/no_untrackedcache_no_fsmonitor
new file mode 100644
index 00000000000..91dc3719cda
--- /dev/null
+++ b/t/t7065/no_untrackedcache_no_fsmonitor
@@ -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)
diff --git a/t/t7065/with_untrackedcache_no_fsmonitor b/t/t7065/with_untrackedcache_no_fsmonitor
new file mode 100644
index 00000000000..91dc3719cda
--- /dev/null
+++ b/t/t7065/with_untrackedcache_no_fsmonitor
@@ -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)
diff --git a/t/t7065/with_untrackedcache_with_fsmonitor b/t/t7065/with_untrackedcache_with_fsmonitor
new file mode 100644
index 00000000000..89d2dd5c2e7
--- /dev/null
+++ b/t/t7065/with_untrackedcache_with_fsmonitor
@@ -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)
diff --git a/wt-status.c b/wt-status.c
index 5813174896c..4dfc8a8969b 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -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

Re: [PATCH v2] status: long status advice adapted to recent capabilities

[Taylor Blau]

On Wed, Nov 02, 2022 at 03:45:18PM -0400, Jeff Hostetler wrote:
> 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:
>
> [...]

Excellent suggestions, thank you.

> > @@ -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", "");
> > +			}
>
> I'm not sure I like the various mixture of messages here.  Maybe
> it would be better with a single simple message:
>
>     _("It took %.2f seconds to enumerate untracked files.\n"
>       "See 'git help status' for information on how to improve this.")
>
> This keeps all of the information in the documentation rather
> than having part of it here in the code.
>
> Also, we should refer to the documentation via `git help` rather
> than as a link to the website.

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

Re: [PATCH v2] status: long status advice adapted to recent capabilities

[Rudy Rigot]

> looking up an unknown configuration variable with 'man
> git-config' is easy enough.

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

> It took %.2f seconds to enumerate untracked files.

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?

> It took %.2f seconds to enumerate untracked files.
> Please look up the core.untrackedCache, core.fsmonitor
> advice.statusUoption, and status.showUntrackedFiles configs
> for potential solutions.

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,

Re: [PATCH v2] status: long status advice adapted to recent capabilities

[Ævar Arnfjörð Bjarmason]

On Thu, Nov 03 2022, Rudy Rigot wrote:

>> looking up an unknown configuration variable with 'man
>> git-config' is easy enough.
>
> 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
>
>> It took %.2f seconds to enumerate untracked files.
>
> 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?
>
>> It took %.2f seconds to enumerate untracked files.
>> Please look up the core.untrackedCache, core.fsmonitor
>> advice.statusUoption, and status.showUntrackedFiles configs
>> for potential solutions.
>
> 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.

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.

Re: [PATCH v2] status: long status advice adapted to recent capabilities

[Rudy Rigot]

> 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 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 took %.2f seconds to enumerate untracked files.
> Try to enable untracked cache to see if it helps make it faster
> for you:
>    git config core.untrackedCache true

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:

> It took %.2f seconds to enumerate untracked files.
> Try to enable FSMonitor to see if it helps make it faster for you:
>    git config core.fsmonitor true

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:

> It took %.2f seconds to enumerate untracked files.
> Your runs are being cached, try running git status again to see if
> it's faster.

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.


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

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.

Re: [PATCH v2] status: long status advice adapted to recent capabilities

[Taylor Blau]

On Thu, Nov 03, 2022 at 09:28:56AM -0500, Rudy Rigot wrote:
> One slightly stronger opinion I have, is that if the advice message
> was just
>
> > It took %.2f seconds to enumerate untracked files.
>
> 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?
>
> > It took %.2f seconds to enumerate untracked files.
> > Please look up the core.untrackedCache, core.fsmonitor
> > advice.statusUoption, and status.showUntrackedFiles configs
> > for potential solutions.
>
> 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.

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

Re: [PATCH v3] status: long status advice adapted to recent capabilities

[Taylor Blau]

On Wed, Nov 02, 2022 at 09:27:47PM +0000, Rudy Rigot via GitGitGadget wrote:
> From: Rudy Rigot <rudy.rigot@gmail.com>
>
> 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 one is looking good to me. Jeff: do you agree? If so, I'm ready to
start merging this one down.

Thanks,
Taylor

Re: [PATCH v3] status: long status advice adapted to recent capabilities

[Derrick Stolee]

On 11/2/22 5:27 PM, Rudy Rigot via GitGitGadget wrote:> +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.

Sorry I'm late to this series. This new section is a great idea.

> +* `-uno` or `status.showUntrackedFiles=false` : just don't search

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.

> +    and don't report on untracked files.  This is the fastest.

  "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.

Perhaps:

	This config option disables a warning message when the
	search for untracked files takes longer than desired.

and maybe even describe why:

	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.

It might be helpful to mention that 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.

It might be worth explicitly mentioning that 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]
> diff --git a/t/t7065-wtstatus-slow.sh b/t/t7065-wtstatus-slow.sh
> new file mode 100755
> index 00000000000..23c37ea71e7
> --- /dev/null
> +++ b/t/t7065-wtstatus-slow.sh
> @@ -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
> diff --git a/t/t7065/no_untrackedcache_no_fsmonitor b/t/t7065/no_untrackedcache_no_fsmonitor
> diff --git a/t/t7065/with_untrackedcache_no_fsmonitor b/t/t7065/with_untrackedcache_no_fsmonitor
> diff --git a/t/t7065/with_untrackedcache_with_fsmonitor b/t/t7065/with_untrackedcache_with_fsmonitor

These files are small enough that I think I'd rather see them
be created in the test script. You can use this kind of syntax:

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

and then the test is self-contained. This is particularly
helpful when a test fails due to some future change in this
message.

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

Since there isn't an "else" for either of these cases, they can
be combined with "&&" in the top-level if, reducing the tab
depth for the body.

> +				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", "");
> +			}
>  		}

other than that nit, the code looks good to me.

Thanks for working on this!
-Stolee

Re: [PATCH v3] status: long status advice adapted to recent capabilities

[Derrick Stolee]

On 11/4/22 5:40 PM, Taylor Blau wrote:
> On Wed, Nov 02, 2022 at 09:27:47PM +0000, Rudy Rigot via GitGitGadget wrote:
>> From: Rudy Rigot <rudy.rigot@gmail.com>
>>
>> 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 one is looking good to me. Jeff: do you agree? If so, I'm ready to
> start merging this one down.

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

Thanks,
-Stolee

Re: [PATCH v3] status: long status advice adapted to recent capabilities

[Eric Sunshine]

On Mon, Nov 7, 2022 at 3:07 PM Derrick Stolee <derrickstolee@github.com> wrote:
> On 11/2/22 5:27 PM, Rudy Rigot via GitGitGadget wrote:> +UNTRACKED FILES AND STATUS SPEED
> > +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
> > +'
> > +
> > diff --git a/t/t7065/no_untrackedcache_no_fsmonitor b/t/t7065/no_untrackedcache_no_fsmonitor
> > diff --git a/t/t7065/with_untrackedcache_no_fsmonitor b/t/t7065/with_untrackedcache_no_fsmonitor
> > diff --git a/t/t7065/with_untrackedcache_with_fsmonitor b/t/t7065/with_untrackedcache_with_fsmonitor
>
> These files are small enough that I think I'd rather see them
> be created in the test script. You can use this kind of syntax:
>
>         cat >expect <<-\EOF
>         <file contents here>
>         EOF &&
>
> and then the test is self-contained. This is particularly
> helpful when a test fails due to some future change in this
> message.

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.

Re: [PATCH v3] status: long status advice adapted to recent capabilities

[Rudy Rigot]

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!

Re: [PATCH v3] status: long status advice adapted to recent capabilities

[Taylor Blau]

On Mon, Nov 07, 2022 at 03:02:09PM -0500, Derrick Stolee wrote:
> On 11/4/22 5:40 PM, Taylor Blau wrote:
> > On Wed, Nov 02, 2022 at 09:27:47PM +0000, Rudy Rigot via GitGitGadget wrote:
> >> From: Rudy Rigot <rudy.rigot@gmail.com>
> >>
> >> 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 one is looking good to me. Jeff: do you agree? If so, I'm ready to
> > start merging this one down.
>
> Sorry, I came in with some late review. I think one more round
> would be helpful.

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


Thanks,
Taylor

[PATCH v4] status: long status advice adapted to recent capabilities

[Rudy Rigot via GitGitGadget]

From: Rudy Rigot <rudy.rigot@gmail.com>

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.

Signed-off-by: Rudy Rigot <rudy.rigot@gmail.com>
---
    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

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 54a4b29b473..c51ba5e79e1 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -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]
diff --git a/t/t7065-wtstatus-slow.sh b/t/t7065-wtstatus-slow.sh
new file mode 100755
index 00000000000..468b8934836
--- /dev/null
+++ b/t/t7065-wtstatus-slow.sh
@@ -0,0 +1,74 @@
+#!/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 &&
+	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.
+
+nothing to commit (create/copy files and use "git add" to track)
+	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.
+
+nothing to commit (create/copy files and use "git add" to track)
+	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
+
+
+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)
+	EOF
+	test_cmp ../expected ../actual &&
+	rm -fr ../actual ../expected
+'
+
+test_done
diff --git a/wt-status.c b/wt-status.c
index 5813174896c..336f41e6d9f 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -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

Re: [PATCH v4] status: long status advice adapted to recent capabilities

[Eric Sunshine]

On Wed, Nov 9, 2022 at 11:46 PM Rudy Rigot via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> 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.
>
> Signed-off-by: Rudy Rigot <rudy.rigot@gmail.com>
> ---
> diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
> @@ -457,6 +457,61 @@ during the write may conflict with other simultaneous processes, causing
> +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

Not necessarily worth a reroll, but the "since we all work..."
fragment could be dropped without any loss of clarity:

    There is no single optimum configuration suitable
    to every situation.

would probably be sufficient.

> +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.

This leaves the reader hanging somewhat by not saying where
"elsewhere" actually is. You can use gitlink: to insert links to the
appropriate documentation.

> +* 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.

This and all the other bullet points use an odd mix of spaces and TAB
for indentation. They should consistently use one or the other.

Earlier in this thread, Ævar suggested that it might be worthwhile to
mention an additional possibility: that simply rerunning `git status`
might be sufficient to achieve reasonable speed since the second run
of `git status` may benefit from the filesystem cache having been
primed by the first invocation of `git status`. As such, it may be
overkill for a user to dive into the various options described here.
Hence, to mitigate the possibility of a user doing a lot of research
and extra unnecessary configuration, it might make sense for the very
first bullet point (before this one) to say merely:

    * Do nothing. Subsequent invocations of `git status` may be faster
      simply because the first `git status` primed the filesystem cache.

or something like that (probably more formal, though).

> diff --git a/t/t7065-wtstatus-slow.sh b/t/t7065-wtstatus-slow.sh
> @@ -0,0 +1,74 @@
> +#!/bin/sh
> +
> +test_description='test status when slow untracked files'
> +
> +. ./test-lib.sh
> +
> +DATA="$TEST_DIRECTORY/t7065"

This variable is unused, isn't it, now that you're inlining everything
with here-docs?

> +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 &&

Rather than asserting that some preceding code has left the
configuration in a state this test wants, it would be more robust for
this test to forcibly set up the state it wants. Something like this
should work:

    test_might_fail git config ---unset-all core.untrackedCache &&
    test_might_fail git config ---unset-all core.fsmonitor &&

> +       git status | sed "s/[0-9]\.[0-9][0-9]/X/g" >../actual &&

At all costs, avoid escaping the "trash" directory in which this test
is running. All files created by this test should be within the
"trash" directory hierarchy. Therefore, drop "../" from all the
filenames, and instead create these files directly in the "trash"
directory or in some subdirectory of the "trash" directory.

I presume the reason you're escaping the "trash" directory is because
you don't want these untracked "actual" and "expected" files to
pollute the `git status` output you're testing? If so, then you should
probably instead set up a .gitignore file in the "trash" directory to
make `git status` ignore them.

We usually want to avoid having a Git command upstream of a pipe since
the exit code of the Git command will be lost. So, we'd typically do
this instead:

    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.
> +
> +nothing to commit (create/copy files and use "git add" to track)
> +       EOF

Two comments. First, use <<-\EOF rather than <<-EOF to make it clear
to readers that you're not interpolating any variables in the here-doc
body. Second, the <<- operator allows you to indent the here-doc body
(with TABs, not spaces), so you can align the body with the rest of
the code:

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

> +       test_cmp ../expected ../actual &&
> +       rm -fr ../actual ../expected
> +'

We usually don't bother cleaning up these files if they don't impact
subsequent tests negatively. However, note that if any code above the
`rm -fr` command fails, then the `rm -fr` command itself won't be run,
hence the files won't get cleaned up upon test failure. To ensure
cleanup (if that's what you desire), use test_when_finished() _before_
code which may fail. So, something like this is typical:

    test_expect_success 'when core.untrackedCache and fsmonitor are unset' '
        test_when_finished "rm -fr actual expected" &&
        test_might_fail git config ---unset-all core.untrackedCache &&
        test_might_fail git config ---unset-all core.fsmonitor &&
        ...
    '

Same comments apply to the other new tests added by this patch.

> diff --git a/wt-status.c b/wt-status.c
> @@ -1205,6 +1207,17 @@ static void wt_longstatus_print_tracking(struct wt_status *s)
> +static inline int uf_was_slow(uint32_t untracked_in_ms)
> +{

Does this need to be inline? Is this a hot piece of code, or is this
merely a premature optimization?

> +       const char *x;
> +       x = getenv("GIT_TEST_UF_DELAY_WARNING");
> +       if (x) {
> +               untracked_in_ms += UF_DELAY_WARNING_IN_MS + 1;
> +       }

Style is to avoid { } braces for these one-liner bodies.

I think we don't even need the variable "x" in this case, so:

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

would be cleaner.

> +       return UF_DELAY_WARNING_IN_MS < untracked_in_ms;
> +}

Re: [PATCH v4] status: long status advice adapted to recent capabilities

[Rudy Rigot]

> the <<- operator allows you to indent the here-doc body
> (with TABs, not spaces), so you can align the body with the rest of
> the code

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:

> I presume the reason you're escaping the "trash" directory is because
> you don't want these untracked "actual" and "expected" files to
> pollute the `git status` output you're testing?

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.

> Does this need to be inline? Is this a hot piece of code, or is this
> merely a premature optimization?

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.

Re: [PATCH v4] status: long status advice adapted to recent capabilities

[Eric Sunshine]

On Thu, Nov 10, 2022 at 12:02 PM Rudy Rigot <rudy.rigot@gmail.com> wrote:
> > the <<- operator allows you to indent the here-doc body
> > (with TABs, not spaces), so you can align the body with the rest of
> > the code
>
> 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.

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.

> > I presume the reason you're escaping the "trash" directory is because
> > you don't want these untracked "actual" and "expected" files to
> > pollute the `git status` output you're testing?
>
> 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.

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
    '

Re: [PATCH v4] status: long status advice adapted to recent capabilities

[Rudy Rigot]

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


On Thu, Nov 10, 2022 at 11:30 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> On Thu, Nov 10, 2022 at 12:02 PM Rudy Rigot <rudy.rigot@gmail.com> wrote:
> > > the <<- operator allows you to indent the here-doc body
> > > (with TABs, not spaces), so you can align the body with the rest of
> > > the code
> >
> > 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.
>
> 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.
>
> > > I presume the reason you're escaping the "trash" directory is because
> > > you don't want these untracked "actual" and "expected" files to
> > > pollute the `git status` output you're testing?
> >
> > 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.
>
> 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
>     '

[PATCH v5] status: long status advice adapted to recent capabilities

[Rudy Rigot via GitGitGadget]

From: Rudy Rigot <rudy.rigot@gmail.com>

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.

Signed-off-by: Rudy Rigot <rudy.rigot@gmail.com>
---
    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

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 5e438a7fdc1..ed1ae3bd35c 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -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]
diff --git a/t/t7065-wtstatus-slow.sh b/t/t7065-wtstatus-slow.sh
new file mode 100755
index 00000000000..bc624756622
--- /dev/null
+++ b/t/t7065-wtstatus-slow.sh
@@ -0,0 +1,68 @@
+#!/bin/sh
+
+test_description='test status when slow untracked files'
+
+. ./test-lib.sh
+
+GIT_TEST_UF_DELAY_WARNING=1
+export GIT_TEST_UF_DELAY_WARNING
+
+test_expect_success setup '
+	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_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.
+
+		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_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.
+
+		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 &&
+	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.
+
+		nothing to commit, working tree clean
+	EOF
+	test_cmp expected actual
+'
+
+test_done
diff --git a/wt-status.c b/wt-status.c
index 5813174896c..5093bf8c894 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -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

Re: [PATCH v3] status: long status advice adapted to recent capabilities

[Jeff Hostetler]

On 11/4/22 5:40 PM, Taylor Blau wrote:
> On Wed, Nov 02, 2022 at 09:27:47PM +0000, Rudy Rigot via GitGitGadget wrote:
>> From: Rudy Rigot <rudy.rigot@gmail.com>
>>
>> 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 one is looking good to me. Jeff: do you agree? If so, I'm ready to
> start merging this one down.
> 
> Thanks,
> Taylor

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

Jeff

Re: [PATCH v5] status: long status advice adapted to recent capabilities

[Jeff Hostetler]

On 11/10/22 3:04 PM, Rudy Rigot via GitGitGadget wrote:
> From: Rudy Rigot <rudy.rigot@gmail.com>
> 
> 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.
> 
> Signed-off-by: Rudy Rigot <rudy.rigot@gmail.com>

I think V5 looks good.
Thanks for your persistence!

Jeff

Re: [PATCH v5] status: long status advice adapted to recent capabilities

[Rudy Rigot]

> I think V5 has addressed all of the concerns.

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.

> Thanks for your persistence!

My absolute pleasure!

Re: [PATCH v5] status: long status advice adapted to recent capabilities

[Eric Sunshine]

On Thu, Nov 10, 2022 at 3:04 PM Rudy Rigot via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> 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.
>
> Signed-off-by: Rudy Rigot <rudy.rigot@gmail.com>

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

> diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
> @@ -457,6 +457,65 @@ during the write may conflict with other simultaneous processes, causing
> +* `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

s/file/files/

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

Might want backticks around the literal filename: `.git/index`

Also: s/cost searching/cost of searching/

> +       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.
> diff --git a/t/t7065-wtstatus-slow.sh b/t/t7065-wtstatus-slow.sh
> @@ -0,0 +1,68 @@
> +test_expect_success setup '
> +       git checkout -b test &&
> +       echo "actual" >> .gitignore &&
> +       echo "expected" >> .gitignore &&
> +       echo "out" >> .gitignore &&

Style: drop space after redirection operator:

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

By the way, this is an excellent opportunity to use a here-doc as
you're now doing elsewhere in the script:

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

Do we want to anchor these .gitignore patterns to make it clear to
future readers the precise expectations of these tests?

    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' '
> +       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.
> +
> +               nothing to commit, working tree clean
> +       EOF

Style: We normally make the here-doc body indentation match the
indentation of the command itself:

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

> +       test_cmp expected actual
> +'
> +
> +test_expect_success 'when core.untrackedCache true, but not fsmonitor' '
> +       git config core.untrackedCache true &&

It's perhaps not super important in this case since each subsequent
test is setting up its configuration exactly as it wants it, but it is
common elsewhere in the test scripts to use test_config() which
automatically unsets the configuration when the test finishes:

    test_config core.untrackedCache true &&

I'm on the fence as to whether or not to use test_config() in this
case, but it shouldn't hurt to do so.

> +       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.
> +
> +               nothing to commit, working tree clean
> +       EOF
> +       test_cmp expected actual
> diff --git a/wt-status.c b/wt-status.c
> @@ -1205,6 +1207,15 @@ static void wt_longstatus_print_tracking(struct wt_status *s)
> +static inline int uf_was_slow(uint32_t untracked_in_ms)

Does this need to be "inline"?

> +{
> +       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;
> +}

Style:

    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;

> @@ -1870,13 +1882,21 @@ static void wt_longstatus_print(struct wt_status *s)
> -               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)) {

Was there a specific reason you switched around the condition so it
checks advice_enabled() _after_ checking for slowness? If so, the
reason may not be obvious to future readers and might deserve mention
in the commit message. If it was just a whim, then future readers
might end up wondering if there was some reason which they are unable
to figure out, in which case it would be better to retain the original
ordering of conditions.

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

To what does "this" refer? Is it this repository? Or something else?

> +                       } 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."));

Okay.

> +                       status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");

Re: [PATCH v5] status: long status advice adapted to recent capabilities

[Rudy Rigot]

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

> Does this need to be "inline"?

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.

> Was there a specific reason you switched around the condition

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

> To what does "this" refer? Is it this repository? Or something else?

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.

Re: [PATCH v5] status: long status advice adapted to recent capabilities

[Eric Sunshine]

On Tue, Nov 15, 2022 at 12:46 PM Rudy Rigot <rudy.rigot@gmail.com> wrote:
> Thanks for the feedback, and I'll integrate it into a new patch, most
> likely today.

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

> > To what does "this" refer? Is it this repository? Or something else?
>
> 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.

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

Re: [PATCH v5] status: long status advice adapted to recent capabilities

[Rudy Rigot]

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

[PATCH v6] status: long status advice adapted to recent capabilities

[Rudy Rigot via GitGitGadget]

From: Rudy Rigot <rudy.rigot@gmail.com>

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.

Signed-off-by: Rudy Rigot <rudy.rigot@gmail.com>
---
    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

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 5e438a7fdc1..570c36e07c1 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -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]
diff --git a/t/t7065-wtstatus-slow.sh b/t/t7065-wtstatus-slow.sh
new file mode 100755
index 00000000000..8d08a962f88
--- /dev/null
+++ b/t/t7065-wtstatus-slow.sh
@@ -0,0 +1,70 @@
+#!/bin/sh
+
+test_description='test status when slow untracked files'
+
+. ./test-lib.sh
+
+GIT_TEST_UF_DELAY_WARNING=1
+export GIT_TEST_UF_DELAY_WARNING
+
+test_expect_success setup '
+	git checkout -b test &&
+	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' '
+	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.
+
+	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_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.
+
+	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 test
+
+	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
diff --git a/wt-status.c b/wt-status.c
index 5813174896c..1f6d64e759f 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -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

Re: [PATCH v6] status: long status advice adapted to recent capabilities

[Eric Sunshine]

On Tue, Nov 15, 2022 at 4:19 PM Rudy Rigot via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> 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.
>
> Signed-off-by: Rudy Rigot <rudy.rigot@gmail.com>
> ---
>     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.

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

> +test_expect_success setup '
> +       git checkout -b test &&
> +       cat >.gitignore <<-\EOF &&
> +       /actual
> +       /expected
> +       /out
> +       EOF
> +       git add .gitignore &&
> +       git commit -m "Add .gitignore"
> +'

I suppose I wasn't paying close enough attention earlier, but I see
now that this is creating a branch named "test". If I remove the `git
checkout -b test` line entirely and change "test" to "master" in the
"expected" files, all the tests pass just fine. So, it's not apparent
why you need to create a specially-named branch here rather than
simply accepting the default branch name. The reason I bring this up
is that unnecessary code, whether in tests or elsewhere, can confuse
future readers (and reviewers, such as myself) into wondering if
something subtle is going on which the reader is overlooking. (This is
very much the same sort of concern as when I asked in an earlier
review why the conditions in an `if` got switched around from the way
they were in the original code; such unnecessary changes can confuse
future readers.)

If the answer is that you wanted to avoid the term "master", then an
alternative would have been to override the default branch name at the
top of the script:

    GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
    export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME

That said, this is minor, and I'm not keen on eating up more of your
time or reviewer time, so I doubt this is worth a reroll.

> +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 &&

When I suggested the above code, it had slipped my mind that we have a
test_unconfig() function in t/test-lib-functions.sh which does this
more concisely:

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

But what you have here in v6 is good enough; it's not worth wasting
your time or reviewer time rerolling just to make this change. I
mention it merely for future reference.

Re: [PATCH v6] status: long status advice adapted to recent capabilities

[Rudy Rigot]

> That said, this is minor, and I'm not keen on eating up more of your
> time or reviewer time, so I doubt this is worth a reroll.

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

> So, it's not apparent
> why you need to create a specially-named branch here rather than
> simply accepting the default branch name.

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.

> an alternative would have been to override the default branch name at the
> top of the script:

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

> we have a test_unconfig() function

I'll use that.

New patch coming!

Re: [PATCH v6] status: long status advice adapted to recent capabilities

[Eric Sunshine]

On Mon, Nov 21, 2022 at 10:55 AM Rudy Rigot <rudy.rigot@gmail.com> wrote:
> > That said, this is minor, and I'm not keen on eating up more of your
> > time or reviewer time, so I doubt this is worth a reroll.
>
> Eh, there's nothing wrong with striving for perfection. Lemme do one
> more reroll...

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)

> > So, it's not apparent
> > why you need to create a specially-named branch here rather than
> > simply accepting the default branch name.
>
> 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.

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".

> > an alternative would have been to override the default branch name at the
> > top of the script:
>
> Oh, this seems like a better way to do what I was trying to do. I'll
> change it now.
>
> > we have a test_unconfig() function
>
> I'll use that.
>
> New patch coming!

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

Re: [PATCH v6] status: long status advice adapted to recent capabilities

[Rudy Rigot]

> 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".

My intuition was that it was something like that indeed.

> you might write the commit message like this

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

> 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

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:

--- expected    2022-11-21 23:46:00.000000000 +0000
+++ actual    2022-11-21 23:46:00.000000000 +0000
@@ -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.

[PATCH v7] status: modernize git-status "slow untracked files" advice

[Rudy Rigot via GitGitGadget]

From: Rudy Rigot <rudy.rigot@gmail.com>

`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.

Signed-off-by: Rudy Rigot <rudy.rigot@gmail.com>
---
    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 <rudy.rigot@gmail.com>
      
       ## 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.
      
          Signed-off-by: Rudy Rigot <rudy.rigot@gmail.com>
      
     @@ 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

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 5e438a7fdc1..570c36e07c1 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -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]
diff --git a/t/t7065-wtstatus-slow.sh b/t/t7065-wtstatus-slow.sh
new file mode 100755
index 00000000000..6733e8ba36b
--- /dev/null
+++ b/t/t7065-wtstatus-slow.sh
@@ -0,0 +1,72 @@
+#!/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"
+'
+
+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 '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 '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_done
diff --git a/wt-status.c b/wt-status.c
index 5813174896c..1f6d64e759f 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -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

Re: [PATCH v6] status: long status advice adapted to recent capabilities

[Eric Sunshine]

On Tue, Nov 22, 2022 at 11:52 AM Rudy Rigot <rudy.rigot@gmail.com> wrote:
> > you might write the commit message like this
>
> The current phrasing was initially copied as is from a past review
> feedback; I have no issues at all replacing it with yours.

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

> > 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
>
> Oh, sorry about that, it hadn't occurred to me that there could be a
> downside to using new test script numbers.

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

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

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

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.

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

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.

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

Perhaps the suggestion I outlined above will help.

Re: [PATCH v6] status: long status advice adapted to recent capabilities

[Eric Sunshine]

On Tue, Nov 22, 2022 at 12:18 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Tue, Nov 22, 2022 at 11:52 AM Rudy Rigot <rudy.rigot@gmail.com> wrote:
> > > you might write the commit message like this
> > - 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.
>
> 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...

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

Re: [PATCH v6] status: long status advice adapted to recent capabilities

[Rudy Rigot]

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

Re: [PATCH v6] status: long status advice adapted to recent capabilities

[Eric Sunshine]

On Tue, Nov 22, 2022 at 12:18 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> 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"
>         )
>     '

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.

Re: [PATCH v6] status: long status advice adapted to recent capabilities

[Eric Sunshine]

On Tue, Nov 22, 2022 at 12:40 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> A minor additional comment if you do go this route and place the new
> tests in an existing script...

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 &&
       ...
    '

Re: [PATCH v6] status: long status advice adapted to recent capabilities

[Rudy Rigot]

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!

Re: [PATCH v6] status: long status advice adapted to recent capabilities

[Eric Sunshine]

On Tue, Nov 22, 2022 at 2:19 PM Rudy Rigot <rudy.rigot@gmail.com> wrote:
> 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.

[PATCH v8] status: modernize git-status "slow untracked files" advice

[Rudy Rigot via GitGitGadget]

From: Rudy Rigot <rudy.rigot@gmail.com>

`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.

Signed-off-by: Rudy Rigot <rudy.rigot@gmail.com>
---
    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(-)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 5e438a7fdc1..570c36e07c1 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -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]
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 2b7ef6c41a4..02d641f0413 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -1676,4 +1676,77 @@ 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 '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 '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 '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
diff --git a/wt-status.c b/wt-status.c
index 5813174896c..1f6d64e759f 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -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

Re: [PATCH v8] status: modernize git-status "slow untracked files" advice

[Junio C Hamano]

"Rudy Rigot via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Rudy Rigot <rudy.rigot@gmail.com>
>
> `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

"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"?

> `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 ...

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

> +UNTRACKED FILES AND STATUS SPEED

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

> +--------------------------------
> +
> +`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.

Good.

> +* 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 above may be a good advice, but it is misleading to make it as
if it is another alternative of equal footing with everything else
listed.  It may likely make the resulting text much easier to follow
if you fold it into "Here is a summary", perhaps like...

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


> +* The `--untracked-files=no` flag or the
> +	`status.showUntrackedfiles=false` config (see above for both) :

Lose the SP before the ":" (applies to all other entries, too).

> +	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.

OK.

> +* `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.

This is not technically wrong per-se, except that "desired" in
"takes longer than desired" may simply be wrong.

The reason why the message may not be a "helpful signal" is in such
a repository and project the user may have already accepted the
current trade-off as _desirable_, iow, the user is WILLING to wait
for 2 seconds.  And in such a case, it indeed is the most sensible
option to disable the advice.

We should also stress the fact that this has nothing to do with
speeding up, unlike other pieces of advice you are giving here.
It's not like disabling the advice will allow us to omit something
we need to do to compute the advice (in other words, if the overhead
to measure the time taken to list untracked files is large, this may
matter, but that is hardly the case).

Perhaps

    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.

or something like that.

> +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 '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 &&

What if it takes more than 10 seconds, e.g.

	"It took 92.34 seconds to enumerate..."

Wouldn't it be redacted into "It took 9X seconds to enumerate"?

It probably does not happen, only because you are forcing the code
to pretend that it took 2.001 seconds or something, I suspect.  But
if you are forcing with GIT_TEST_UF_DELAY_WARNING to pretend that it
took some unacceptably long time, it may be more robust to

 * pass "struct wt_status *s" to uf_was_slow(), instead of passing
   s->untracked_in_ms

 * when GIT_TEST_UF_DETAIL_WARNING tells us we are pretending a long
   delay for the purpose of running tests, ASSIGN a known value to
   s->untracked_in_ms

 * get rid of "out" and use of "sed" in these test, and instead
   check for exact output.

e.g.

	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;
	}

plus

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

Also, what do you need /g modifier in "sed" script for?  I do not
think we give more than one such number in the message we are
testing.

> +		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.

This is not wrong per-se, but it is more customary to do say:

		See '\''git help status'\'' for information on ...

All of the comments for this test apply to other two new tests.

> +		nothing to commit, working tree clean
> +		EOF
> +		test_cmp expected actual
> +	)
> +'

Additionally (read: you do not _have_ to do this to make this topic
acceptable, but it probably is worth thinking about), if we need to
introduce a new helper function uf_was_slow() anyway, a much better
change may be to make the 2 seconds cut-off configurable, than
inventing GIT_TEST_UF_DETAIL_WARNING used only for tests.  You can
introduce, say, "status.enumerateUntrackedDelayMS", a configuration
variable that can be set to override the hardcoded 2000 milliseconds
(i.e. UF_DELAY_WARNING_IN_MS) to control what delay is acceptable
for the repository.

Then you can run the tests with the configuration set to a negative
value (i.e. no time is acceptably short, even 0 milliseconds).  If
you go that route, then you do need to redirect to "out" and redact
with "sed" (make sure you are prepared to see a delay more than 10
seconds in such a case).

Thanks.

Re: [PATCH v8] status: modernize git-status "slow untracked files" advice

[Rudy Rigot]

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


> "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"?

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.


> Additionally (read: you do not _have_ to do this to make this topic
> acceptable, but it probably is worth thinking about), if we need to
> introduce a new helper function uf_was_slow() anyway, a much better
> change may be to make the 2 seconds cut-off configurable, than
> inventing GIT_TEST_UF_DETAIL_WARNING used only for tests.

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.


> Wouldn't it be redacted into "It took 9X seconds to enumerate"?
> It probably does not happen, only because you are forcing the code
> to pretend that it took 2.001 seconds or something, I suspect.

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


> Also, what do you need /g modifier in "sed" script for?  I do not
> think we give more than one such number in the message we are
> testing.

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!

Re: [PATCH v8] status: modernize git-status "slow untracked files" advice

[Rudy Rigot]

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.

[PATCH v9] status: modernize git-status "slow untracked files" advice

[Rudy Rigot via GitGitGadget]

From: Rudy Rigot <rudy.rigot@gmail.com>

`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.

Signed-off-by: Rudy Rigot <rudy.rigot@gmail.com>
---
    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(-)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 5e438a7fdc1..a051b1e8f38 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -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]
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 2b7ef6c41a4..aed07c5b622 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -1676,4 +1676,74 @@ 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 '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 >actual &&
+		cat >expected <<-\EOF &&
+		On branch main
+
+		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
+		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 >actual &&
+		cat >expected <<-\EOF &&
+		On branch main
+
+		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
+		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 >actual &&
+		cat >expected <<-\EOF &&
+		On branch main
+
+		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.
+
+		nothing to commit, working tree clean
+		EOF
+		test_cmp expected actual
+	)
+'
+
 test_done
diff --git a/wt-status.c b/wt-status.c
index 5813174896c..b430d25da43 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -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

Re: [PATCH v9] status: modernize git-status "slow untracked files" advice

[Junio C Hamano]

"Rudy Rigot via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Rudy Rigot <rudy.rigot@gmail.com>
>
> `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.
>
> Signed-off-by: Rudy Rigot <rudy.rigot@gmail.com>
> ---
>     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.

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.

Re: [PATCH v9] status: modernize git-status "slow untracked files" advice

[Rudy Rigot]

> (I thought you were going to update the proposed
> log message, too, though)

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!

Re: [PATCH v9] status: modernize git-status "slow untracked files" advice

[Junio C Hamano]

Rudy Rigot <rudy.rigot@gmail.com> writes:

>> (I thought you were going to update the proposed
>> log message, too, though)
>
> 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:

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

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.

Re: [PATCH v9] status: modernize git-status "slow untracked files" advice

[Rudy Rigot]

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!

Re: [PATCH v8] status: modernize git-status "slow untracked files" advice

[Eric Sunshine]

On Tue, Nov 22, 2022 at 5:07 PM Rudy Rigot via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> `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.
>
> Signed-off-by: Rudy Rigot <rudy.rigot@gmail.com>
> ---
>     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.

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 &&
            ...
        )
    '