Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid excessive file system watches for --server #1465

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

chklauser
Copy link

@chklauser chklauser commented Feb 1, 2025

Fixes #1468 (dotnet-csharpier --server accidentally installs file system watches for the entire directory.)

We configure the server to run against a temporary, empty directory instead, so that the file system watcher that does get installed only has to watch a single, empty directory.

This also means that the server won't pick up the default appsettings.json files in the working directory. We probably don't want this anyway because these appsettings could be intended for the user's solution, not for csharpier.

Alternative approaches are discussed in #1468

@chklauser

This comment was marked as resolved.

Editor plugins, like the Rider extension, run the server in the root directory
of a solution. Using the root directory as the content root for an ASP.NET application
is a bad idea, because the default host setup installs a recursive file system watch on the entire
directory. This file system watch does not honor any ignore files and will quickly exhaust OS resource
with large solutions/multiple server processes.

We thus configure the server to run against a temporary, empty directory instead.

This also means that the server won't pick up the default appsettings.json files in the working directory.
We probably don't want this anyway because these appsettings could be intended for the user's solution,
not for csharpier.
@chklauser chklauser force-pushed the avoid-server-fs-watches branch from b278dd4 to ebcb4dd Compare February 1, 2025 11:35
@belav
Copy link
Owner

belav commented Feb 2, 2025

Thanks for the thorough explanation of the issue!

I wasn't sure how the file watcher uses the project directory as the content root and had the thought that a possible fix would be to set the working directory of the process to the same directory that the csharpier process exists in.

rider does not set a working directory at all. So if the default behavior of rider is to run the process from the project root that could explain how it ends up as the content root.

vscode does specifically set the project directory as the working directory when starting the process. That was changed in #543 but I'm not exactly sure why it was needed.

I think it's worth checking if setting the working directory to the same directory as the csharpier process solves this problem. It means the issue can be fixed by just updating the extensions. When a file is formatted the path of that file is passed to the server, so I am fairly certain we can get anything we need using that path.

If that doesn't work I'd be inclined have the extensions disable csharpier server on linux for versions without this fix.

@chklauser
Copy link
Author

Yes, I can confirm that it is the working directory that gets used as the default content root (checked with strace). So if the extensions would set the process working directory to the same directory as the dotnet-csharpier executable, the file system watches wouldn't be that much of a problem.

I still see a bunch watches for directories in the ~/.cache/csharpier/$version/ directory, .store with different subdirectories for languages and .net versions. Unnecessary work still happens. But at least those directories won't experience massive changes (such as git checkouts) while you are working.

But I ultimately don't have a strong opinion on how exactly the issue gets fixed. I have created an issue #1468 with the problem description above (in case there are going to be multiple PRs).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Excessive file system watches for --server
2 participants