microsoft / scalar

Scalar: A set of tools and extensions for Git to allow very large monorepos to run on Git without a virtualization layer
MIT License
1.39k stars 63 forks source link

VS Code interacts poorly with Watchman-based fsmonitor, causing many Git commands running in background #446

Closed hbinkle closed 3 years ago

hbinkle commented 4 years ago

Hi, since I registered my react project for scalar I noticed that every 5 seconds a bunch of git commands is done in background for every file opened in Visual Studio code. Such a bunch is done every 5 seconds for every opened file:

> git status -z -u
> git symbolic-ref --short HEAD
> git rev-parse [...]
> git rev-parse --symbolic-full-name [...]@{u}
> git rev-list --left-right [...]...refs/remotes/origin/[...]
> git for-each-ref --sort -committerdate --format %(refname) %(objectname)
> git remote --verbose
> git config --get commit.template

This slows down vscode significantly when there are several files open.

Unregistering the project from scalar does not help. Only when removing the git config entries scalar adds the problem is solved:

       [...]
    multiPackIndex = true
    preloadIndex = true
    safecrlf = false
    untrackedCache = false
    hookspath = C:/project/.git/hooks
    fsmonitor = C:/project/.git/hooks/query-watchman
    fsmonitorHookVersion = 2

ENV:

derrickstolee commented 4 years ago

Thanks for the report, @hbinkle. This is definitely an unintended interaction, and I'm confused why VS Code is running these commands so often.

Are you willing to do some digging to find out exactly which config setting is causing this? Basically the approach I would take is to take the list of Scalar-set config settings and use a binary search to find which one(s) are causing this issue: disable only half at a time and see if the issue persists. Use that to find hopefully exactly one that is causing the issue.

To assist, the config settings for the latest release are here: "required" (but actually optional for scalar register) and "optional" for all repos.

Some should absolutely not be at fault, such as core.multiPackIndex=true. Others, like core.fsmonitor might be somehow causing a problem, but my feeling is not.

My gut feeling is that something like status.aheadBehind=false is at fault.

hbinkle commented 4 years ago

@derrickstolee the wathcman enty is causing it: fsmonitor = C:/project/.git/hooks/query-watchman as soon as I remove it vscode behaves as usual.

will scalar work without that entry properly or should I un-register to avoid other strange behavior?

derrickstolee commented 4 years ago

@derrickstolee the wathcman enty is causing it: fsmonitor = C:/project/.git/hooks/query-watchman as soon as I remove it vscode behaves as usual.

will scalar work without that entry properly or should I un-register to avoid other strange behavior?

Scalar will work without the fsmonitor feature. (In fact, it will not configure it if you do not have Watchman installed.)

THANK YOU @hbinkle for discovering the bad configuration value, and I will make sure we investigate why this feature is interacting badly with VS Code!

I hope you don't mind, but I'm going to rename this issue to track the real cause.

dscho commented 4 years ago

IIRC Watchman writes some "cookie" file whenever it receives a query. Or maybe it is Git that is writing that cookie? If this is indeed so, then I bet that VS Code's own filesystem monitor claims that a file was added and deleted, triggers a git status, which of course would then trigger another cookie file to be written, lather, rinse and repeat.

In any case, is there a chance to run Process Monitor while the issue occurs and then have a look at the files created in the worktree? That could potentially give us a further indication whether my hypothesis is correct or not.

derrickstolee commented 4 years ago

IIRC Watchman writes some "cookie" file whenever it receives a query. Or maybe it is Git that is writing that cookie? If this is indeed so, then I bet that VS Code's own filesystem monitor claims that a file was added and deleted, triggers a git status, which of course would then trigger another cookie file to be written, lather, rinse and repeat.

That's a good point. VS Code is also watching the filesystem to check for changes, so I think this cycle is happening:

  1. VS code runs git status
  2. git status calls the FS Monitor hook.
  3. Watchman writes a cookie file, then returns the results to git status.
  4. VS Code sees that cookie file write, so thinks the working directory changed.
  5. VS Code runs git status... (repeat)
dscho commented 4 years ago

From looking at https://github.com/microsoft/vscode/blob/f74e473238aca7b79c08be761d99a0232838ca4c/src/vs/workbench/api/browser/mainThreadFileSystemEventService.ts#L34, it does not appear to me as if there are any special file names excluded from the file watching in VS Code :-(

github-actions[bot] commented 3 years ago

Labeling this issue as stale. There has been no activity for 30 days. Remove stale label or comment or this issue will be closed in 7 days.

dscho commented 3 years ago

From looking at https://github.com/microsoft/vscode/blob/f74e473238aca7b79c08be761d99a0232838ca4c/src/vs/workbench/api/browser/mainThreadFileSystemEventService.ts#L34, it does not appear to me as if there are any special file names excluded from the file watching in VS Code :-(

Actually, I think that there is code in VSCode that prevents anything inside .git/ from being acted on: https://github.com/microsoft/vscode/blob/1.52.1/extensions/git/src/repository.ts#L861

So we should be good on the VS Code side, as long as the cookie file is created within .git/.

dscho commented 3 years ago

Actually, I think that there is code in VSCode that prevents anything inside .git/ from being acted on: https://github.com/microsoft/vscode/blob/1.52.1/extensions/git/src/repository.ts#L861

It's also possible that it is https://github.com/microsoft/vscode/blob/1.52.1/extensions/git/src/repository.ts#L543 instead.

jeffhostetler commented 3 years ago

The comment here: https://github.com/microsoft/vscode/blob/1.52.1/extensions/git/src/repository.ts#L879 implies that VSCode will wake up for any changes within the .git directory, so if we put fsmonitor cookie files within .git directory itself, we cause the problem to continue.

While working on https://github.com/gitgitgadget/git/pull/923 I noticed this behavior too. Moving the cookie files into .git/foo/ did not fix the problem. (I assume it is because the mtime on foo/ changed each time a cookie file was created/deleted within.)

Moving the cookie files into .git/foo/bar/ did fix the problem.

I have not looked at the watchman-related scripts.

dscho commented 3 years ago

I guess this is actually addressed as part of v2, which made it into Git for Windows v2.32.0-rc2.

jeffhostetler commented 3 years ago

I updated the builtin fsmonitor daemon code to put the cookie file deeper in the .git directory and confirmed that that prevented VSCode from overreacting when the daemon was active.

I need to independently fix and/or test the Watchman hook-based version.

github-actions[bot] commented 3 years ago

Labeling this issue as stale. There has been no activity for 30 days. Remove stale label or comment or this issue will be closed in 7 days.

dscho commented 3 years ago

Oh wow, this was addressed some time ago now...