WriteCharsLegacy performance issues caused by NotifyConsoleUpdateSimpleEvent #14356

@lhecker commented on GitHub (Jun 29, 2021):

Hmm, running accevent.exe (and thus enabling UIA) doesn't change the performance for me much either... So why does @skyline75489's trace show such a high CPU time in NotifyConsoleUpdateSimpleEvent?

@lhecker commented on GitHub (Jun 29, 2021): Hmm, running `accevent.exe` (and thus enabling UIA) doesn't change the performance for me much either... So why does @skyline75489's trace show such a high CPU time in `NotifyConsoleUpdateSimpleEvent`?

claunia commented

@DHowett commented on GitHub (Jun 29, 2021):

This reminds me a bit of https://github.com/microsoft/terminal/issues/410#issuecomment-485012574, but it's a simple event and not a sizing event... hm.

@DHowett commented on GitHub (Jun 29, 2021): This reminds me a bit of https://github.com/microsoft/terminal/issues/410#issuecomment-485012574, but it's a simple event and not a sizing event... hm.

claunia commented

@DHowett commented on GitHub (Jun 29, 2021):

To be fair, however: enabling any event causes us to send all events, due to the architecture of the legacy accessibility eventing system.

@DHowett commented on GitHub (Jun 29, 2021): To be fair, however: enabling _any_ event causes us to send _all_ events, due to the architecture of the legacy accessibility eventing system.

claunia commented

@DHowett commented on GitHub (Jun 29, 2021):

Interesting. We do not disable accessibility notification in ConPTY. Are we wasting cycles doing this when we're sitting behind Terminal (or Code, or something else) too?

@DHowett commented on GitHub (Jun 29, 2021): Interesting. We do not disable accessibility notification in ConPTY. Are we wasting cycles doing this when we're sitting behind Terminal (or Code, or something else) too?

claunia commented

@lhecker commented on GitHub (Jun 29, 2021):

@DHowett I was already wondering why we're posting MSAA events, when WT already has it's own UIA integration.
So basically, if I understand you correctly, we can disable MSAA entirely for ConPTY and in our OpenConsole build (which unlike conhost is only used by WT), right?

@lhecker commented on GitHub (Jun 29, 2021): @DHowett I was already wondering why we're posting MSAA events, when WT already has it's own UIA integration. So basically, if I understand you correctly, we can disable MSAA entirely for ConPTY and in our OpenConsole build (which unlike conhost is only used by WT), right?

claunia commented

@DHowett commented on GitHub (Jun 29, 2021):

I'd rather disable it specifically when we're hosting a PTY -- we don't want to be sending those events when we're doing ConPTY using the Windows inbox infrastructure either :)

@DHowett commented on GitHub (Jun 29, 2021): I'd rather disable it specifically when we're hosting a PTY -- we don't want to be sending those events when we're doing ConPTY using the Windows inbox infrastructure either :)

claunia commented

@DHowett commented on GitHub (Jun 29, 2021):

Chester may also just be testing openconsole directly. It's a good testbed for the raw performance of one round of VT parsing with no VT string formatting.

@DHowett commented on GitHub (Jun 29, 2021): Chester may also just be testing openconsole directly. It's a good testbed for the raw performance of one round of VT parsing with no VT string formatting.

claunia commented

2026-01-31 04:08:10 +00:00

@skyline75489 commented on GitHub (Jun 29, 2021):

@DHowett is right. I was testing only OpenConsole. The screenshot shot and the 3x boost come from running OpenConsole directly.

@skyline75489 commented on GitHub (Jun 29, 2021): @DHowett is right. I was testing only OpenConsole. The screenshot shot and the 3x boost come from running OpenConsole directly.

claunia commented

@skyline75489 commented on GitHub (Jun 30, 2021):

This is how it looks like with WT:

Basically the same thing as @lhecker posted.

@skyline75489 commented on GitHub (Jun 30, 2021): This is how it looks like with WT: ![image](https://user-images.githubusercontent.com/4710575/123886428-0226b880-d982-11eb-846e-89a560755b4a.png) Basically the same thing as @lhecker posted.

claunia commented

2026-01-31 04:08:10 +00:00

@skyline75489 commented on GitHub (Jul 2, 2021):

So the ConPTY part is obviously fixable and profitable as shown in #10537. Are we do something about the OpenConsole part of this issue? That's a painfully obvious performance bottleneck that I think I first noticed in the year of 2020.

@skyline75489 commented on GitHub (Jul 2, 2021): So the ConPTY part is obviously fixable and profitable as shown in #10537. Are we do something about the OpenConsole part of this issue? That's a painfully obvious performance bottleneck that I think I first noticed in the year of 2020.

claunia commented

2026-01-31 04:08:10 +00:00

@DHowett commented on GitHub (Jul 2, 2021):

We can only make it faster; we cannot remove it, as there are applications with a dependency on our legacy accessibility events.

@DHowett commented on GitHub (Jul 2, 2021): We can only make it faster; we cannot remove it, as there are applications with a dependency on our legacy accessibility events.

claunia commented

@skyline75489 commented on GitHub (Jul 2, 2021):

We can only make it faster; we cannot remove it, as there are applications with a dependency on our legacy accessibility events.

😢

I don't have enough knowledge to make it faster without breaking other applications. Maybe someone can shine some light on this particular subject, when they have the time.

@skyline75489 commented on GitHub (Jul 2, 2021): > We can only make it faster; we cannot remove it, as there are applications with a dependency on our legacy accessibility events. 😢 I don't have enough knowledge to make it faster without breaking other applications. Maybe someone can shine some light on this particular subject, when they have the time.

claunia commented

@miniksa commented on GitHub (Jul 6, 2021):

If we think of accessibility like a renderer to a screen reader... then it may be acceptable to coalesce the events coming out of it and skip frames or only report some of the information over the MSAA channel since no one could reasonably be expected to "read" all of that information anyway.

However, node.js and presumably other things globally hook MSAA events in some versions as a way of getting notifications that the buffer data has changed and scraping all of its context out of it... because it took us a really long time to provide something like ConPTY and that was their only option. They DO want to "read" ALL of the information, so skipping frames doesn't work for them.

I think the only way out of this box is through ConPTY and you're not going to fix it for conhost. You could choose to fix it for your own personal OpenConsole where you don't need the MSAA events... or we could add some sort of MSAA-opt-out flag on startup of conhost that turns them off if you know you don't need them... but not by default in the OS. The strategy there is to just migrate away from conhost as the headed environment.

@miniksa commented on GitHub (Jul 6, 2021): If we think of accessibility like a renderer to a screen reader... then it may be acceptable to coalesce the events coming out of it and skip frames or only report some of the information over the MSAA channel since no one could reasonably be expected to "read" all of that information anyway. However, node.js and presumably other things globally hook MSAA events in some versions as a way of getting notifications that the buffer data has changed and scraping all of its context out of it... because it took us a really long time to provide something like ConPTY and that was their only option. They DO want to "read" ALL of the information, so skipping frames doesn't work for them. I think the only way out of this box is through ConPTY and you're not going to fix it for conhost. You could choose to fix it for your own personal OpenConsole where you don't need the MSAA events... or we could add some sort of MSAA-opt-out flag on startup of conhost that turns them off if you know you don't need them... but not by default in the OS. The strategy there is to just migrate away from conhost as the headed environment.

claunia commented

@DHowett commented on GitHub (Jul 6, 2021):

Node we know about -- it uses it solely for resize, which we really should address by having a better resize channel.

It's the nebulous "other things" that we can't know. :)

@DHowett commented on GitHub (Jul 6, 2021): Node we know about -- it uses it solely for resize, which we really should address by having a better resize channel. It's the nebulous "other things" that we can't know. :)

claunia commented