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

VS2017 RC - Colorization causing slow editing in large files #1821

Closed
dsyme opened this issue Nov 23, 2016 · 4 comments · Fixed by #1829
Closed

VS2017 RC - Colorization causing slow editing in large files #1821

dsyme opened this issue Nov 23, 2016 · 4 comments · Fixed by #1829
Labels
Area-LangService-API Bug Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. Regression

Comments

@dsyme
Copy link
Contributor

dsyme commented Nov 23, 2016

If you open a large file (e.g. TypeChecker.fs or CompileOps.fs) then character-by-character editing is noticably slow, much slower than in VS2012-15.

The underlying problem is large CPU time usage by the routine AddSyntacticClassificationsAsync. Here are the top entries for a profiling run for 20 seconds of editing (this was using a debug build of Visual F# Tools, but the same results should apply to a release build )

Function Name	Total CPU (ms)	Total CPU (%)	Self CPU (ms)	Self CPU (%)	Module
 - <StartupCode$FSharp-Editor>.$ColorizationService+Microsoft-CodeAnalysis-Editor-IEditorClassificationService-AddSyntacticClassificationsAsync@138::Invoke	14055	48.06%	0	0.00%	FSharp.Editor.dll
| - Microsoft.VisualStudio.FSharp.Editor.FSharpColorizationService::GetColorizationData	14011	47.91%	139	0.48%	FSharp.Editor.dll
|| - Microsoft.VisualStudio.FSharp.Editor.FSharpColorizationService::scanSourceLine	12726	43.52%	140	0.48%	FSharp.Editor.dll
||| - <StartupCode$FSharp-Editor>.$ColorizationService+scanAndColorNextToken@62::Invoke	11465	39.20%	219	0.75%	FSharp.Editor.dll
|||| - Microsoft.FSharp.Compiler.SourceCodeServices.FSharpLineTokenizer::ScanToken	10861	37.14%	527	1.80%	FSharp.LanguageService.Compiler.dll
||||| + <StartupCode$FSharp-LanguageService-Compiler>.$ServiceLexing+GetTokenWithPosition@569::Invoke	4487	15.34%	349	1.19%	FSharp.LanguageService.Compiler.dll
||||| + <StartupCode$FSharp-LanguageService-Compiler>.$ServiceLexing+FinalState@675::Invoke	2141	7.32%	11	0.04%	FSharp.LanguageService.Compiler.dll
||||| + Microsoft.FSharp.Compiler.SourceCodeServices.LexerStateEncoding::decodeLexInt	1025	3.50%	170	0.58%	FSharp.LanguageService.Compiler.dll

Repro steps

  1. Open VisualFSharp.sln and go to COmpileOps.fs ot TYpeChecker,fs

  2. Randomly delete characters and revert changes

  3. Repeat this

  • at the start of large files in projects
  • at the end of large files in projects
  • at the start of large scripts
  • at the end of large scripts
  • at the start of large files out of projects
  • at the end of large files out of projects

Expected behavior

Fast editing

Actual behavior

Slow editing

Known workarounds

Use small code files :)

@dsyme
Copy link
Contributor Author

dsyme commented Nov 23, 2016

@OmarTawfik I have a fix for this. Colorization is slow when the document text is changing in large documents. My understanding is that the table holding the FSharpColorization SourceTextData should be keyed by document or documentID (which doesn't change), rather than by document source text (which changes)

@dsyme
Copy link
Contributor Author

dsyme commented Nov 24, 2016

Fix ready as part of #1829

@dsyme dsyme added Area-LangService-API Regression Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. Bug labels Nov 24, 2016
dsyme added a commit that referenced this issue Nov 24, 2016
Fixes #1821

Incremental colorization was not being effective on changing files for a number of reasons
•Data cache was keyed by source text rather than document ID
•We were writing None entries right through to end-of-file even when we could stop earlier
•We weren't checking that start-of-line indexes and lex states were still the same when reusing entries
•Tokenizers getting recreated for each line - we can cache these as well.

I've tried the PR out on large files and it works much more efficiently.

Also a fix to make  ShouldTriggerCompletionAux  faster in the common case where not pressing  . 

This also fixes glyphs: #1806. Public/private/protected is not yet propagated to the glyph but this gives us feature parity with VS2015
@Pilchie
Copy link
Member

Pilchie commented Nov 28, 2016

FSharpColorization SourceTextData should be keyed by document or documentID (which doesn't change), rather than by document source text (which changes)

For reference Document does change - it is the immutable snapshot of the file. DocumentId is the stable identifier that lets you look up Documents in different snapshots.

@dsyme
Copy link
Contributor Author

dsyme commented Nov 28, 2016

@Pilchie Thanks for reviewing these post-hoc. Can confirm we switched to DocumentID here https://github.com/Microsoft/visualfsharp/pull/1829/files#diff-de5bdafb119bed995860757094ee328eR61

nosami pushed a commit to xamarin/visualfsharp that referenced this issue Jan 26, 2022
Fixes dotnet#1821

Incremental colorization was not being effective on changing files for a number of reasons
•Data cache was keyed by source text rather than document ID
•We were writing None entries right through to end-of-file even when we could stop earlier
•We weren't checking that start-of-line indexes and lex states were still the same when reusing entries
•Tokenizers getting recreated for each line - we can cache these as well.

I've tried the PR out on large files and it works much more efficiently.

Also a fix to make  ShouldTriggerCompletionAux  faster in the common case where not pressing  . 

This also fixes glyphs: dotnet#1806. Public/private/protected is not yet propagated to the glyph but this gives us feature parity with VS2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-LangService-API Bug Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. Regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants