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

Load referenced grammars lazily #106

Open
mjbvz opened this issue Aug 15, 2019 · 3 comments
Open

Load referenced grammars lazily #106

mjbvz opened this issue Aug 15, 2019 · 3 comments
Assignees
Labels
feature-request Request for new features or functionality

Comments

@mjbvz
Copy link
Contributor

mjbvz commented Aug 15, 2019

From microsoft/vscode#77990

Problem
When loading a grammar that references other grammars, we currently always pull in the referenced grammars eagerly. For languages like markdown that reference a large number of grammars, this can be quite expensive.

Proposed Fix
If possible, we should investigate to see if we can resolve these referenced grammars lazily instead. For example, the markdown grammar should only load the pug grammar when it actually encounters a pug fenced code block while tokenizing text

/cc @alexandrudima

@mjbvz
Copy link
Contributor Author

mjbvz commented Aug 16, 2019

After debugging through the code, I think we could look into making getExternalGrammar both load and init external grammars. Right now, we eagerly load the external grammars in Registry._loadGrammar and then getExternalGrammar lazily loads it

This may be problematic however as RegistryOptions.loadGrammar returns a promise and getExternalGrammar is called from a fairly low level that is not async

@alexdima
Copy link
Member

alexdima commented Aug 16, 2019

I think this is a nice thing to have, but consider that tokenizeLine/tokenizeLine2 are synchronous methods and loading grammars is asynchronous (in VS Code it is wired to go through the IFileService), so IMHO this is not something straight-forward to do. Are there other editors that use TM and which load grammars lazily?

So far, this has been a limitation that was well understood and that impacted only markdown. In fact, I wrote a fast plist parser especially for markdown due to this and at the time the impact for loading markdown was in the 100ms-200ms range. See microsoft/vscode#8173 .

IMHO, I think the markdown performance and the TS performance should be treated as two separate issues, because one was well understood and accepted since years and is caused by really needing to load a lot of other languages, and the other one was (unnecessarily) introduced via injection to TypeScript.

@mjbvz
Copy link
Contributor Author

mjbvz commented Aug 16, 2019

Yeah I realized this is too far down in the stack to easily make async. We can fix the regression for js/ts since this is not simple. I just wanted to make sure we know that the root cause of the problem is not js/ts specific

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality
Projects
None yet
Development

No branches or pull requests

2 participants