-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
perf(lsp): add built-in tracing support for the LSP #27843
base: main
Are you sure you want to change the base?
Conversation
62347e3
to
b83c056
Compare
} | ||
for (const oldEntry of LANGUAGE_SERVICE_ENTRIES.byScope.values()) { | ||
oldEntry.ls.dispose(); | ||
const span = ops.op_make_span(`serverRequest(${method})`, true); |
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.
This diff is huge - maybe rename this function to serverRequestInner
and then add serverRequest
that wraps it in the span and try/finally?
"getScriptVersion", | ||
"fileExists", | ||
"getScriptSnapshot", | ||
"getCompilationSettings", | ||
"getCurrentDirectory", | ||
"useCaseSensitiveFileNames", | ||
"getModuleSpecifierCache", | ||
"getGlobalTypingsCacheLocation", | ||
"getSourceFile", |
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.
What's the reason for excluding these? Too noisy?
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.
I think we should upstream these to our TS fork
No description provided.