evilC / AutoHotInterception

An AutoHotkey wrapper for the Interception driver
MIT License
707 stars 64 forks source link

Would it be possible to use interception_wait instead of a 1ms polling loop, to reduce CPU usage? #75

Open AlexVallat opened 3 years ago

AlexVallat commented 3 years ago

Has this already been tried and found to be unhelpful for some reason? Otherwise, I see in the example code for Interception that interception_wait is used to block until an event occurs, then interception_receive to get the event that just occurred. This would probably be more CPU friendly than polling every 1ms.

AlexVallat commented 3 years ago

As an experiment, I've converted ScanCodeChecker to use Wait instead of polling in my fork: https://github.com/AlexVallat/AutoHotInterception/commit/e00264da4dd7ed351fd3c807599159115e9d0e47

This uses WaitWithTimeout to allow for clean shutdown, but if you aren't concerned about being able to stop waiting for a key without killing the process then Wait could be used instead.

evilC commented 3 years ago

TBH I am really not sure, I certainly don't recall ever investigating interception_wait I just had a play (https://github.com/evilC/AutoHotInterception/tree/interception_wait) and it seems to work just fine (Only tested for Subscription mode so far, but I am wondering if this would even conceivably fix the timing issues with context mode)

evilC commented 3 years ago

Updated ScanCodeChecker in interception_wait branch to use waiting without modifying ManagedWrapper BTW are you in the Discord channel?

AlexVallat commented 3 years ago

No, not in the discord channel, sorry - I can get it set up for tomorrow afternoon if you will be around at a similar time and would like to discuss?

I'll take a look at your new branch, thanks. I think ManagedWrapper ought to be updated in any case as the WaitWithTimeout signature is just incorrect.

I haven't used Context mode, so not sure about the timing issues, is that #8 ?

evilC commented 3 years ago

What I meant by that was that context mode is generally glitchy. I am not sure I have any scripts which repro issues, it's been so long since I looked at it, but certainly I remember a bunch of people reporting issues with it. It's pretty quirky how it works - basically it fires a callback to the AHK script which sets a context variable in AHK (Thus enabling a context-specific hotkey), then sends the key, then fires the callback to AHK again which then sets the context flag back so the hotkey is disabled again. However, yeah, it would certainly be worth revisiting the Sleep 0 issue again to see if it's no longer required with this new technique. Wrt to the WaitWithTimeout signature, entirely possible - hell, I didn't even write the managed wrappings myself (I am pretty clueless about such things), I forget even where I got them. I'll take your word for it - have updated it in my branch

AlexVallat commented 3 years ago

For the ManagedWrapper I referred to https://github.com/oblitum/Interception/blob/v1.0.1/library/interception.h#L176 as the source of truth; as it hasn't been updated since 2017 I think it highly unlikely to be different from the compiled dll.

Looking at the Sleep 0 thing, I think this is appropriate and required. Going through the execution flow, your code intercepts the key, sets the flag, then sends the key. This will make windows put the WM_KEYDOWN message in the queue for AHK, but it's not going to block the Send method waiting for AHK to processes that message. So immediately you will call the callback to clear the context flag, and this would probably happen before the WM_KEYDOWN is processed. Putting the Sleep 0 in before the context flag is cleared tells AHK to process any pending messages (including the WM_KEYDOWN) first.

There are probably other ways to handle this, but it doesn't seem too hacky to me.

AlexVallat commented 3 years ago

I've taken a fork of interception_wait and modified it to use the same WaitTimeout I did for the ScanCodeCheker experiment, should fix the issue with SetState. I also changed it to a Thread rather than ThreadPool work item as this thread is intended to stick around long term. PR here, if you want it: #76

An alternative would be to not remove the filter before doing the SetThreadState, but do it afterwards - that way incoming messages would break it out of the Wait. The problem with that, though, is if the user has only subscribed to a device which doesn't often get input then blocking until the next input wouldn't be great. SetState is probably rare enough that it doesn't matter if it is slow to return - for faster enabling and disabling I would expect Subscribe/Unsubscribe to be better.

AlexVallat commented 3 years ago

Another idea I've had, we can actually replicate the interception_wait code from the Interception dll ourselves, which allows adding another wait handle to wait on. This can be a cancellation token wait handle, which means we don't need a timeout at all, it will automatically interrupt when the cancellation token is signalled. Check it out in https://github.com/AlexVallat/AutoHotInterception/tree/WaitWithoutTimeout

evilC commented 2 years ago

A big thankyou @AlexVallat for alerting me to this Interception feature. I have now released AHI 0.6.0 which implements WaitWithTimeout and a CancellationToken I have not yet updated the ScanCodeChecker to use WaitWithTimeout, but quite possibly will do so. The next update I am planning is to address issues with Pause / Numlock, and Extended Key Code handling in general. In order to do this, I will probably be updateing the ScanCodeChecker to help me properly get a handle on this.