Skip to content
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

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

Open
AlexVallat opened this issue May 27, 2021 · 9 comments

Comments

@AlexVallat
Copy link

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
Copy link
Author

As an experiment, I've converted ScanCodeChecker to use Wait instead of polling in my fork: AlexVallat@e00264d

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
Copy link
Owner

evilC commented May 27, 2021

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
Copy link
Owner

evilC commented May 27, 2021

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

@AlexVallat
Copy link
Author

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
Copy link
Owner

evilC commented May 27, 2021

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
Copy link
Author

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
Copy link
Author

AlexVallat commented May 28, 2021

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
Copy link
Author

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
Copy link
Owner

evilC commented Jan 14, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants