Filter out project early if it doesn't have any formattable documents

[erdalsivri]

Improves the runtime in large solutions when a small number of files is included (--include). This is a common use-case for developers to lint/format only modified files in their branch.

In a solution with about 80 projects, this improves the runtime by about 50% when --include SomeProject/SomeFile.cs is specified.

Related issues: #1378 and #757

[erdalsivri]

Can add tests if someone gives a thumbs-up for the approach. Didn't want to waste time adding tests before making sure what I am trying to do here makes sense

[siriak]

Thanks a lot, waiting for it to be merged :)

[erdalsivri]

@JoeRobich do you mind taking a look at this PR? I couldn't find a way to a request review directly

[JoeRobich]

@erdalsivri Thanks for opening this PR. I think this is a good change. We could probably even expand it so that we build an ImmutableArray up front and pass that into the Formatters so that even whitespace formatting could take advantage of this.

[erdalsivri]

@erdalsivri Thanks for opening this PR. I think this is a good change. We could probably even expand it so that we build an ImmutableArray up front and pass that into the Formatters so that even whitespace formatting could take advantage of this.

Sounds good! There are few other small performance improvements I have been working on. May send a few more PRs your way.

We'd really like to see dotnet format (at least the whitespace command) as fast as go fmt but I couldn't find an easy way to get rid of the cost of building a Microsoft.CodeAnalysis.Workspace and adding a dummy Solution to it, which take almost 2s in our solution :( Refactoring the code so that FolderWorkspace doesn't depend on Workspace doesn't look like a trivial change so I didn't attempt it.

I understand that running analyzers is slow because they require a build but not all style checks require a build. For example, "Add braces to 'if' statement" (https://docs.microsoft.com/pt-pt/dotnet/fundamentals/code-analysis/style-rules/ide0011) requires dotnet format style, which requires a restore and build (because it is implemented as an analyzer?), but this shouldn't require a build; having the SyntaxTree should be enough to fix it if I am not mistaken. We should be able to write a very fast document formatter for this, right?

This is the output (cropped) of dotnet format style --verify-no-changes --verbosity diag --include SomeProject/SomeFile.cs on our solution (on this branch):

 Loading workspace.
  ...
 Complete in 30903ms.
  Determining formattable files.
  Complete in 554ms.
  Running formatters.
 Running Code Style analysis.
  Determining diagnostics...
  Running 7 analyzers on SomeProject.
/SomeProject/SomeFile.cs(44,5): error IDE0011: Add braces to 'if' statement. [/SomeProject.csproj]
  Complete in 12118ms.
  Analysis complete in 12119ms.
  Complete in 12637ms.
  Formatted code file 'SomeProject/SomeFilecs'.
  Formatted 1 of 3939 files.
  Format complete in 44096ms.

It even knows I have 3939 files in my solution but it doesn't need to. I guess that's why loading the workspace takes 30s. Even if we ignore the time spent on loading the workspace, formatting itself takes about 10 seconds. Anyways, maybe I should create a bug about these. There is probably not an easy fix for these though

[erdalsivri]

Looks like I don't have permission to merge even with an approval. @JoeRobich are you going to merge this one?

[JoeRobich]

I couldn't find an easy way to get rid of the cost of building a Microsoft.CodeAnalysis.Workspace

Part of that is filesystem related. We have to query to find potential source files and applicable .editorconfig files. If you are seeing time being elsewhere, please open an issue and we can dig into it more.

I understand that running analyzers is slow because they require a build

We discussed internally building a workspace cache server or perhaps providing a way to rehydrate a workspace from a MSBuild Binary Log.

but not all style checks require a build.

This is another enhancement that we've tossed around internally. Keeping a static list of diagnostics and fixers that can work from syntax and do not require semantic information.

We are excited for this fix to get in. Thanks for submitting it.

[erdalsivri]

I couldn't find an easy way to get rid of the cost of building a Microsoft.CodeAnalysis.Workspace

Part of that is filesystem related. We have to query to find potential source files and applicable .editorconfig files. If you are seeing time being elsewhere, please open an issue and we can dig into it more.

Yes, .editorconfig exploration takes quite some time in our solution (even with dotnet format whitespace) due to a huge node_modules folder, which is of course totally unrelated to the C# build. I have a branch that removes this step but I am seeing 1.8s even without EditorConfigFinder.GetEditorConfigPaths. Will create a bug with details. In short, the slowness is due to MefHostServices.DefaultHost and OnSolutionAdded both of which outside of dotnet-format repo

I understand that running analyzers is slow because they require a build

We discussed internally building a workspace cache server or perhaps providing a way to rehydrate a workspace from a MSBuild Binary Log.

Is there a bug/FR that I can follow? I can create one if not

but not all style checks require a build.

This is another enhancement that we've tossed around internally. Keeping a static list of diagnostics and fixers that can work from syntax and do not require semantic information.

That sounds good! Is there a bug/FR for this one?

We are excited for this fix to get in. Thanks for submitting it.