-
Notifications
You must be signed in to change notification settings - Fork 117
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
Add grammar performance warning #128
Conversation
This code is executed in the browser context, so there is no need in perf_hooks module |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Yanpas
Thank you for your PR. But:
- This node module needs to execute in browsers and in nodejs, so we cannot use
perf_hooks
. - We cannot just console.warn with a somewhat random time limit. VS Code ships to millions of devices with different specifications, and such a performance problem should be investigated by the grammar author, and no cycles should be spent to record duration on end users machines. In other words, IMHO, it is not useful to run this troubleshooting code all the time.
That being said, perhaps the Registry or the tokenize call can get some kind of option called "profile: true" or something. Then, only if that option is set, the times will be recorded. Then, we can adopt this in VS Code and check if running from source and then enable that flag.
|
|
So I see no other option as proposed above |
Updated branch |
Thank you! |
Inspired by microsoft/vscode#92369