Starting with a TS/JS file is significantly file slower than opening other files #77990

jrieken · 2019-07-26T13:06:52Z

start profiler
reload with a TS file
stop profiler
🔥 notice how all grammar files are read and processed
start profiler again
reload with a JSON file
stop profiler
🚀 getting colors was much, much quicker

The problem is the jsdoc injection which itself drags in the MD grammar which drags in the world...

alexdima · 2019-07-29T14:06:37Z

root cause --

vscode/extensions/typescript-basics/package.json

Lines 81 to 90 in c94b270

    
           { 
        
             "scopeName": "documentation.injection", 
        
             "path": "./syntaxes/jsdoc.injection.tmLanguage.json", 
        
             "injectTo": [ 
        
               "source.ts", 
        
               "source.tsx", 
        
               "source.js", 
        
               "source.js.jsx" 
        
             ] 
        
           }

This injection drags in markdown, which drags in almost all our grammars...

Removing that reduces the time significantly.

mjbvz · 2019-08-13T21:09:39Z

I believe the core reason for this is the markdown grammar's fenced code block support. That ends up pulling in grammars for most of our builtin languages. Will see if including a subset of the markdown grammar fixes this or if we maybe need to split up the markdown grammar file in some way

(note that we already do include subsets of the markdown grammar instead of pulling in the entire thing, but one of those subsets is the entire fenced code block set of rules)

alexdima · 2019-08-14T10:16:35Z

@mjbvz Good idea! I think going for a reduced subset is the right thing to do.

I would also incorporate this into the TS grammar itself, the TS grammar already appears to do a lot of JSDoc parsing anyways, so why wouldn't Sublime and other editors where the TS grammar is used benefit from this...

Also, IMHO we should avoid injection if possible and in this case I think it is possible because we can reach to the TS team who own the TS grammar and they can add the fenced block rule to their grammar if we would give them a PR.

mjbvz · 2019-08-14T19:03:32Z

It's a bit complicated since fenced code blocks can appear inside other markdown elements, like lists, which we do want to support in jsdoc comments

Sublime doesn't support some of the rules we need to support the jsdoc highlighting feature (microsoft/TypeScript-TmLanguage#693) so that why I went with the injection based approach

I think we either need to:

Inline some basic parts of the markdown grammar
Split all the fenced code block into an injection grammar (these are not loaded for referenced grammars)

I'll be looking into the first approach today

mjbvz · 2019-08-15T23:09:04Z

After thinking about this more, I don't think we should do a js/ts specific fix. The same performance problem also exists when you load VS Code with a markdown file open.

I've opened microsoft/vscode-textmate#106 that proposes loading referenced grammars lazily. This would fix the issue for js/ts and for normal md files too

alexdima · 2019-08-16T07:44:19Z

@mjbvz Let's remove the TS injection until we address microsoft/vscode-textmate#106
(We have only added this a few months ago and IMHO is not critical to the TS experience)

bpasero · 2019-10-28T08:23:08Z

I am still seeing this in web. Steps:

add a console.log statement where we load grammars using file service:

vscode/src/vs/workbench/services/textMate/browser/abstractTextMateService.ts

Line 195 in 2843554

const content = await this._fileService.readFile(resource);

open a JS/TS file
observe that every grammar seems to be loaded

javascript/syntaxes/JavaScript.tmLanguage.json
typescript-basics/syntaxes/jsdoc.js.injection.tmLanguage.json
markdown-basics/syntaxes/markdown.tmLanguage.json
typescript-basics/syntaxes/TypeScript.tmLanguage.json
javascript/syntaxes/JavaScriptReact.tmLanguage.json
css/syntaxes/css.tmLanguage.json
html/syntaxes/html.tmLanguage.json
ini/syntaxes/ini.tmLanguage.json
java/syntaxes/java.tmLanguage.json
lua/syntaxes/lua.tmLanguage.json
make/syntaxes/make.tmLanguage.json
perl/syntaxes/perl.tmLanguage.json
r/syntaxes/r.tmLanguage.json
ruby/syntaxes/ruby.tmLanguage.json
php/syntaxes/php.tmLanguage.json
sql/syntaxes/sql.tmLanguage.json
vb/syntaxes/asp-vb-net.tmlanguage.json
xml/syntaxes/xml.tmLanguage.json
xml/syntaxes/xsl.tmLanguage.json
yaml/syntaxes/yaml.tmLanguage.json
bat/syntaxes/batchfile.tmLanguage.json
clojure/syntaxes/clojure.tmLanguage.json
coffeescript/syntaxes/coffeescript.tmLanguage.json
cpp/syntaxes/c.tmLanguage.json
cpp/syntaxes/cpp.tmLanguage.json
git/syntaxes/diff.tmLanguage.json
docker/syntaxes/docker.tmLanguage.json
git/syntaxes/git-commit.tmLanguage.json
git/syntaxes/git-rebase.tmLanguage.json
go/syntaxes/go.tmLanguage.json
groovy/syntaxes/groovy.tmLanguage.json
pug/syntaxes/pug.tmLanguage.json
javascript/syntaxes/Regular%20Expressions%20(JavaScript).tmLanguage
json/syntaxes/JSON.tmLanguage.json
json/syntaxes/JSONC.tmLanguage.json
less/syntaxes/less.tmLanguage.json
objective-c/syntaxes/objective-c.tmLanguage.json
swift/syntaxes/swift.tmLanguage.json
scss/syntaxes/scss.tmLanguage.json
perl/syntaxes/perl6.tmLanguage.json
powershell/syntaxes/powershell.tmLanguage.json
python/syntaxes/MagicPython.tmLanguage.json
python/syntaxes/MagicRegExp.tmLanguage.json
rust/syntaxes/rust.tmLanguage.json
shellscript/syntaxes/shell-unix-bash.tmLanguage.json
typescript-basics/syntaxes/TypeScriptReact.tmLanguage.json
csharp/syntaxes/csharp.tmLanguage.json
fsharp/syntaxes/fsharp.tmLanguage.json
handlebars/syntaxes/Handlebars.tmLanguage.json
log/syntaxes/log.tmLanguage.json
html/syntaxes/html-derivative.tmLanguage.json
typescript-basics/syntaxes/jsdoc.ts.injection.tmLanguage.json
cpp/syntaxes/cpp.embedded.macro.tmLanguage.json
scss/syntaxes/sassdoc.tmLanguage.json

alexdima · 2019-10-28T10:27:37Z

It is unfortunate how this was never verified because it didn't have a bug label :|

alexdima · 2019-10-28T14:38:19Z

The fix from @mjbvz in 5c27385 unfortunately had no effect because the grammar resolver was not written to only partially resolve grammars, so even if a small subset of markdown was included via injection, the entirety of it was loaded.

bpasero · 2019-10-28T14:52:19Z

Thanks!

mjbvz · 2019-10-29T01:57:57Z

Thanks @alexandrudima! Sorry, I thought I had profiled when I made the original fix but clearly was not look at the right thing. You change looks much better

bpasero · 2019-10-29T07:27:09Z

@mjbvz there are other languages that will suffer from this issue too, should we fix them as well? For example I think F#, PHP, Perl. Probably anything that depends on markdown.

connor4312 · 2019-10-29T20:56:28Z

I gave a spin at verifying this by measuring the total time spent in tokenize2 for a small JSON file and a small TypeScript file. Understanding that TypeScript will always be more complex than JSON, here's the results:

JSON:
 - 18.5ms
 - 20.6ms
 - 16.8ms

TypeScript:
 - 72.4ms
 - 76.1ms
 - 82.0ms

This appears a good bit better than the ~300ms Johannes reported initially, though we're on different hardware and opening different files. @alexandrudima @jrieken are these measurements within the bounds that you'd consider this resolved? (Unlabel if not 🙂 )

jrieken · 2019-10-30T07:54:38Z

Yeah, I have measure the same number. I think reasonable because the JSON grammar is trivial in comparison to the TS grammar. Tho, there is still #77990 (comment) which we should probably move into a separate issue

bpasero · 2019-10-30T08:01:21Z

Opened #83633 for @mjbvz

alexdima · 2019-11-06T14:42:07Z

This is all rather complicated, but we had to revert vscode-textmate to the last month's stable version, because of a flaw which was causing #83541 .

That would technically mean that this issue is reopened (with the revert).

The lowest risk fix at this point is to revert vscode-textmate and delete the problematic injections.

jrieken assigned alexdima and mjbvz Jul 26, 2019

jrieken added the perf-startup label Jul 26, 2019

alexdima added the important Issue identified as high-priority label Jul 29, 2019

alexdima modified the milestones: July 2019, August 2019 Jul 29, 2019

kieferrm mentioned this issue Aug 10, 2019

Iteration Plan for August 2019 #78844

Closed

37 tasks

matter123 mentioned this issue Aug 15, 2019

Syntax highlighting for doxygen comments jeff-hykin/better-cpp-syntax#353

Open

mjbvz mentioned this issue Aug 15, 2019

Load referenced grammars lazily microsoft/vscode-textmate#106

Open

mjbvz closed this as completed in 5c27385 Aug 20, 2019

vscodebot bot locked and limited conversation to collaborators Oct 4, 2019

bpasero reopened this Oct 28, 2019

bpasero added the verification-needed Verification of issue is requested label Oct 28, 2019

bpasero modified the milestones: August 2019, October 2019 Oct 28, 2019

alexdima added the bug Issue identified by VS Code Team member as probable bug label Oct 28, 2019

alexdima closed this as completed in microsoft/vscode-textmate@9aafda4 Oct 28, 2019

connor4312 added verified Verification succeeded and removed verification-needed Verification of issue is requested labels Oct 29, 2019

alexdima added the candidate Issue identified as probable candidate for fixing in the next release label Nov 6, 2019

alexdima added verification-needed Verification of issue is requested and removed verified Verification succeeded labels Nov 6, 2019

mjbvz added the verified Verification succeeded label Nov 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Starting with a TS/JS file is significantly file slower than opening other files #77990

Starting with a TS/JS file is significantly file slower than opening other files #77990

jrieken commented Jul 26, 2019

alexdima commented Jul 29, 2019

mjbvz commented Aug 13, 2019 •

edited

Loading

alexdima commented Aug 14, 2019

mjbvz commented Aug 14, 2019 •

edited

Loading

mjbvz commented Aug 15, 2019

alexdima commented Aug 16, 2019

bpasero commented Oct 28, 2019 •

edited

Loading

alexdima commented Oct 28, 2019

alexdima commented Oct 28, 2019

bpasero commented Oct 28, 2019

mjbvz commented Oct 29, 2019

bpasero commented Oct 29, 2019

connor4312 commented Oct 29, 2019 •

edited

Loading

jrieken commented Oct 30, 2019

bpasero commented Oct 30, 2019

alexdima commented Nov 6, 2019

Starting with a TS/JS file is significantly file slower than opening other files #77990

Starting with a TS/JS file is significantly file slower than opening other files #77990

Comments

jrieken commented Jul 26, 2019

alexdima commented Jul 29, 2019

mjbvz commented Aug 13, 2019 • edited Loading

alexdima commented Aug 14, 2019

mjbvz commented Aug 14, 2019 • edited Loading

mjbvz commented Aug 15, 2019

alexdima commented Aug 16, 2019

bpasero commented Oct 28, 2019 • edited Loading

alexdima commented Oct 28, 2019

alexdima commented Oct 28, 2019

bpasero commented Oct 28, 2019

mjbvz commented Oct 29, 2019

bpasero commented Oct 29, 2019

connor4312 commented Oct 29, 2019 • edited Loading

jrieken commented Oct 30, 2019

bpasero commented Oct 30, 2019

alexdima commented Nov 6, 2019

mjbvz commented Aug 13, 2019 •

edited

Loading

mjbvz commented Aug 14, 2019 •

edited

Loading

bpasero commented Oct 28, 2019 •

edited

Loading

connor4312 commented Oct 29, 2019 •

edited

Loading