-
Notifications
You must be signed in to change notification settings - Fork 8.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
Skip accessibility notifier and all event calculations when we're in PTY mode #10569
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -76,6 +76,15 @@ try | |
} | ||
} | ||
|
||
if (args->InConptyMode()) | ||
{ | ||
// If we're in ConPTY mode... set the no-op accessibility notifier | ||
// to prevent sending expensive legacy MSAA events when we won't even be | ||
// responsible for the terminal user interface. | ||
std::unique_ptr<IAccessibilityNotifier> noOpNotifier = std::make_unique<Microsoft::Console::Interactivity::NoOpAccessibilityNotifier>(); | ||
RETURN_IF_NTSTATUS_FAILED(ServiceLocator::SetAccessibilityNotifier(std::move(noOpNotifier))); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm surprised this doesn't cause us to blow up later... because isn't somebody else going to set it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, it creates it if it was not already created. Clever. |
||
} | ||
|
||
// Removed allocation of scroll buffer here. | ||
return S_OK; | ||
} | ||
|
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.
We may want to audit the places where IAccessibilityNotifier is used, in addition to having the no-op interface.
I say this because ... the one that notifies about buffer changes reads the buffer first, before calling out to IAN. It's a waste of time if IAN is a no-op, which still increases our output time burden.
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.
meh, you've got a good point about
SCREEN_INFORMATION::NotifyAccessibilityEventing
. If we're really trying to be as optimal as possible, then yea we shouldn't do that.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'm wondering why
SCREEN_INFORMATION::NotifyAccessibilityEventing
is the only thing showed up in the WPR trace. Maybe this indicates that other places that callsIAccessibilityNotifier
are not as expensive?