Typing extremely slow when editing .editorconfig file

[AmadeusW]

Version Used:
16.5.0 preview 3.0 [29818.62.d16.5] ⚠
16.6.0 preview 1.0 [29818.10.master]

Steps to Reproduce:

1. Open .editorconfig file
2. Type in it

Expected Behavior:

There is no lag. It's a simple text file

Actual Behavior:

There is a huge lag. VS seems unresponsive
VS has impact on the entire system which appears slow
VS may crash
PerfView trace reveals that OnAnalyzerConfigDocumentOpened takes up 96% of inclusive samples

module microsoft.codeanalysis.workspaces.ni <<microsoft.codeanalysis.workspaces.ni!Microsoft.CodeAnalysis.Workspace+<>c.<OnAnalyzerConfigDocumentOpened>b__189_2(Microsoft.CodeAnalysis.Workspace, Microsoft.CodeAnalysis.DocumentId, Microsoft.CodeAnalysis.Text.SourceText, Microsoft.CodeAnalysis.PreservationMode)>>

Dump is available at \\amadeus\perf\editorconfig slow on corpnet

[mavasani]

Tagging @sharwell @jasonmalinowski if they have seen this.

[jasonmalinowski]

So it wouldn't surprise me that background processing might be slow since a change to an .editorconfig requires us to reparse all files (potentially). But OnAnalyzerConfigDocumentOpened wouldn't be where I'd expect to see that CPU time.

[sharwell]

When an editorconfig file is opened, or when a character is typed, we discard the entire Roslyn solution state and start over. It is the worst possible case while editing. I think last time I checked there was a limit of about 55 comment characters typed before the IDE just can't keep up anymore and it crashes.

[CyrusNajmabadi]

Had a good conversation with @agocke about the challenges here, especially about the compiler/compilation needing to eagerly know up front per document about all the new config file info.

We tentatively spitballed an approach where instead of this being per-tree info, it was instead per-compilation info exposed through a deferred computation model. i.e. something like:

CompilationOptions
{
   CompilationOptions(..., SyntaxTreeOptionsProvider provider) { ... }
   ImmutableDictionary<string, ReportDiagnostic> GetOptionsForTree(SyntaxTree tree) => _provider.GetOptionsForTree(tree);
}
abstract class SyntaxTreeOptionsProvider
{
    public ImmutableDictionary<string, ReportDiagnostic> GetOptionsForTree(SyntaxTree tree)
}

We would also have the IDE no longer pass in this information into trees, allowing us to just reuse the tree instances directly.

The benefit here is that we could easily just fork the compilation (which we do anyways) while keeping all data except the SyntaxTreeOptionsProvider. Most compilations wouldn't even need to access this as most would just be immediately ignored once the next user change came in. Then, only when it was necessary to process those trees woudl the compiler call bback into us to get this data.

One downside I can see immediately is that this would be a synchronous call back into us. though it looks like we have that issue with other providers (like MetadataReferenceResolver). And perhaps we can asynchronously compute the new provider, and queue up the next compilation when that is done.

Another downside is that this changes the observable Compilation/Tree surface area for existing consumers. i.e. because we no longer passed this information into the trees, if an existing consumer was looking at the tree for this dictionary, they would now need to go to the compilation. We would have to pass this through compat, but i think it's a totally reasonable step for the expected perf gains we would get.

[CyrusNajmabadi]

Passing over to compiler. @agocke feel free to update this as appropriate to go through your triage system.

[sharwell]

I'm interested to see the exact API surface changes proposed. If the public API stays the same then it sounds like changing it could be fine.

• Have we considered using the approach of project files (only process changes on save)?
• Have we considered using the approach of Single File Generators (only process changes on save and/or switching to a different focused document)?
• Have we verified that under the current approach, when the syntax trees are recreated we at least reuse the green nodes?

[agocke]

Ran into a problem while implementing this: I forgot that we also pull the generation status from the editorconfig.

I'm looking at how viable it is to yank this out as well.

[CyrusNajmabadi]

what is "generation status"? thanks!

[agocke]

Whether or not the file is marked "generated" which ends up informing the starting nullable state

[CyrusNajmabadi]

wacky. how does editor config play into that??

[mavasani]

@CyrusNajmabadi: https://docs.microsoft.com/visualstudio/code-quality/use-roslyn-analyzers#configure-generated-code. This editorconfig option is also respected by compiler's nullable analysis. It is populated here: http://sourceroslyn.io/#Microsoft.CodeAnalysis.CSharp/CommandLine/CSharpCompiler.cs,220

[jasonmalinowski]

This was addressed with #44331.