-
Notifications
You must be signed in to change notification settings - Fork 565
IDE: Operation-specific metrics and small improvements #2500
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
Conversation
…pecific timer. Used to measure the latency of specific operations.
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.
LGTM!
main/lsp/LSPMessage.cc
Outdated
void LSPMessage::cancel() { | ||
if (!canceled) { | ||
canceled = true; | ||
// Protect against nullptrs. |
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.
Personal preference nit: although I'm generally in favor of more comments, this specific one didn't give me any extra information that wasn't clear to me from the code. (I see that this code moved, and the original had this comment, so just giving you the liberty to remove if you found it equally tame.)
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.
Ah, I can add context: A Timer may not be specified (it's optional).
why is this requesting extra reviews? sorry about that |
I think dmitry changed some things. |
IDE: Operation-specific metrics and small improvements
Specific changes:
LSPMessage
s now have a second optional timer that tracks the latency of specific methods.LSPMessage
now contains message (and timer) cancelation logic.enableMultithreading
is now a property ofLSPTask
rather than a parameter toTypecheckerCoordinator::syncRun
.TypecheckerTask::hasDedicatedThread
toTypecheckerTask::collectCounters
.Motivation
We have no way to check the latency of specific operations in SignalFX, as all operations are grouped together
Test plan
See included automated tests.