Performance degradation in Windows consoles caused by nvdaHelper #622

Closed
nvaccessAuto opened this Issue Apr 12, 2010 · 8 comments

2 participants

@nvaccessAuto

Reported by jteh on 2010-04-12 03:39
Windows text consoels use EVENT_CONSOLE_* winEvents. These events are always delivered out-of-context, even if you set a winEvent hook for in-context events. Currently, nvdaHelper hooks all winEvents, including these. These events cause some minor performance degradation, which is quite noticeable when, for example, displaying several thousand lines of text to a console. This is made worse because nvdaHelper 32, nvdaHelper 64 and NVDA itself are all listening for these events. There's absolutely no reason for nvdaHelper to listen to them, so we should stop doing so.

@nvaccessAuto

Comment 2 by jteh on 2010-05-11 06:47
The larger part of this performance degradation was actually due to nvdaHelperRemoteLoader not pumping its message queue, so it was receiving a lot of out-of-context events but never processing them.
Changes:
Changed title from "nvdaHelper: Don't hook EVENT_CONSOLE_*" to "Performance degradation in Windows consoles caused by nvdaHelper"

@nvaccessAuto

Comment 3 by jteh on 2010-05-11 21:56
Fixed in a9c859f.
Changes:
State: closed

@nvaccessAuto

Comment 4 by aleksey_s on 2010-07-27 08:43
It is in fact fixed in 2010.2 code cicle.
Changes:
Milestone changed from 2010.3 to 2010.2

@nvaccessAuto

Comment 5 by jteh on 2010-08-24 09:20
Couldn't decide whether to reopen this or file a regression ticket. Decided to reopen because it's in the same release. :)

Unfortunately, nvdaHelperRemoteInjectionFix regressed this for Windows 7. It appears that in Windows 7, you can inject into conhost.exe, but you can't inject into the thread which dispatches the console win events. We assumed that registering for events only from the current process meant that we would never get out-of-context win events. However, in this case, the thread (not the process) prevents injection, so we get these events out-of-context. Our inProcMgrThreadFunc doesn't have a message loop, so we have the same problem as before.

I think the best solution is to again avoid registering for the console win events, since these seem to be special. We could also make inProcThreadMgrFunc have a message loop (using !MsgWaitForMultipleObjects), but even if we do this, I think it's best to avoid these out-of-context events altogether.
Changes:
State: reopened

@nvaccessAuto

Comment 6 by jteh (in reply to comment 5) on 2010-08-24 09:48
Replying to jteh:

I think the best solution is to again avoid registering for the console win events, since these seem to be special.

I split the registration into two ranges: 0-0x4000 and 0x4100-0xffffffff (which is what we had previously). Unfortunately, this doesn't fix it! Interestingly, if I don't register the higher range, the lag goes away. This doesn't make sense to me, since the higher range definitely doesn't include any console events. I also checked and we're not receiving any out-of-context events with this change. I'm stumped.

@nvaccessAuto

Comment 7 by jteh (in reply to comment 6) on 2010-08-24 21:48
Replying to jteh:

I also checked and we're not receiving any out-of-context events with this change.

Err... I obviously screwed up when I checked. We do receive out-of-context events other than EVENT_CONSOLE_* from conhost. First of all, we're receiving events above EVENT_MAX. I've no idea how this is possible, but we shouldn't register for anything higher than EVENT_MAX anyway. We're also receiving EVENT_OBJECT_LOCATIONCHANGE, probably for the caret.

This means that even if we do avoid registering for console events, we still need the message loop to avoid lagging the other out-of-context events. I tested this and it fixes the problem. I still think we should avoid registering for console events, though, as we should avoid as many out-of-context events as possible. My testing seems to show a slight speed improvement doing this, though it's hard to tell whether I'm imagining it. In any case, I have code which does both.

An alternative is to avoid injecting into conhost altogether, as it should be the only thing that ever fires out-of-context events.

@nvaccessAuto

Comment 8 by jteh on 2010-08-24 22:57
Crap. My code misbehaves because inThreadInjectionID relies on there being only one win event hook ID. Not sure how to fix this yet.

@nvaccessAuto

Comment 9 by jteh on 2010-08-25 03:30
Fixed in 88ebf13. We don't prevent registration of console win events, as this seems to provide little to no performance boost and would be extremely complicated to do; see my previous comment.
Changes:
State: closed

@nvaccessAuto nvaccessAuto added this to the 2010.2 milestone Nov 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment