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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
jrieken opened this issue Jul 26, 2019 · 6 comments

Comments

@jrieken
Copy link
Member

commented Jul 26, 2019

  • 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

Screenshot 2019-07-26 at 15 01 52

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

@alexandrudima

This comment has been minimized.

Copy link
Member

commented Jul 29, 2019

root cause --

{
"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.

@alexandrudima alexandrudima modified the milestones: July 2019, August 2019 Jul 29, 2019

@kieferrm kieferrm referenced this issue Aug 10, 2019
25 of 37 tasks complete
@mjbvz

This comment has been minimized.

Copy link
Contributor

commented Aug 13, 2019

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)

@alexandrudima

This comment has been minimized.

Copy link
Member

commented Aug 14, 2019

@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

This comment has been minimized.

Copy link
Contributor

commented Aug 14, 2019

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

This comment has been minimized.

Copy link
Contributor

commented Aug 15, 2019

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

@alexandrudima

This comment has been minimized.

Copy link
Member

commented Aug 16, 2019

@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)

@mjbvz mjbvz closed this in 5c27385 Aug 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can鈥檛 perform that action at this time.