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