[Issue] CPU usage/polling

[braoult]

The CPU used by g910-gkeys looks pretty high, even when doing nothing.

On my system, top shows a regular 4% usage, and never goes below 3.5%.

Running strace(1) on the process, it appears the 2 following lines are repeating forever:

select(0, NULL, NULL, NULL, {tv_sec=0, tv_usec=1000}) = 0 (Timeout)
timerfd_settime(11, TFD_TIMER_ABSTIME, {it_interval={tv_sec=0, tv_nsec=0}, it_value={tv_sec=107179, tv_nsec=221910000}}, NULL) = 0

Would it be possible to avoid polling and use blocking IO ?

Thabks,

br.

[JSubelj]

Will look this up when I have the time.

[hbregalad]

Blocking io might be better, but control = dev.read(endpoint.bEndpointAddress, endpoint.wMaxPacketSize, USB_TIMEOUT) is already probably already blocking enough for our purposes. increasing, USB_TIMEOUT from 5 ms to 50 ms or even 5000 ms in usb_and_keyboard_device_init.py

setting USB_TIMEOUT = 50 ms drops my CPU usage from 3%-6% down to .3%. Comfortably similar to the other 6 tasks that are always using a more than 0.

setting USB_TIMEOUT = 500 ms drops the CPU usage to close enough to 0 for my purposes.

setting USB_TIMEOUT = 5000 ms proves the only thing that looses responsiveness is config.json reloads. Response time on gkey macros is still the same, and g810-LED color changes are not impacted.

I suggest that 1 to 4 times a second is fast enough response time for detecting config.json changes. Dictating a value of 1000 ms to 250 ms.

[suabo]

Would it be possible to avoid polling and use blocking IO ?

How I understand it right now libusb is handling the device like a filestream and we are constantly reading from it. To act on a command we received, we need to stop reading and parse the bytes received from the stream. After that it repeats. Reading gets stopped if bytes to read or the timeout is reached.

How is it now On reading we need to set the bytes to read and a timeout. We take the max packet size which is defined by the hardware itself, 64 bytes in my case. Therefore the timeout is set to a small value to catch packets with less than 64 bytes i guess.

If I look at the debug output on my device i see a value bInterval on the Endpoint which means the keyboard is at top speed capable to send/receive commands every millisecond. Now it is set to 5ms (usb timeout).

Improvements It would be cool, if we could set the number of bytes to read to 20 so we could raise the timeout to infinite. That way, we wouldn't run the loop every millisecond. But the media keys only got 2 bytes so i don't think it's gonna work. I'm gonna keep testing a bit more here and maybe also refactor the layout_config_helpers to just ouput the raw byte codes of keys.

Blocking io might be better, but control = dev.read(endpoint.bEndpointAddress, endpoint.wMaxPacketSize, USB_TIMEOUT) is already probably already blocking enough for our purposes.

I did testing on this and the reading on the device is blocked by the timeout, so i see no reason to halt the process with sleep so I removed it in my testing branch and couldn't mention any back drops. The timeout will define a responsiveness of the keys. I got pretty close to zerp cpu usage with a timeout set to 100 ms. 5ms is for most use cases not needed. Even hitting the key 10 times in a second would be impossible for me, but that's what 100 ms timeout means at the end.

setting USB_TIMEOUT = 5000 ms proves the only thing that looses responsiveness is config.json reloads. Response time on gkey macros is still the same, and g810-LED color changes are not impacted.

I need to look into this because maybe there is 64 bytes send all the time and we skip x00 at the beginning, what means we really could raise the timeout to infinite. I can't see why the usb timeout is related to the signal send by filesystem?

[braoult]

Looks good.

I am sorry, I don't know Python, but should I understand there is no equivalent to C's select() or poll() system calls ? This is the usual way to handle non-blocking read() in C (as we can know that something is available on a file descriptor). You can then read (non-blocking) as long as you receive bytes; when a read returns nothing, you can simply loop again to select() / poll().

[hbregalad]

My testing seemed to indicate that reads would return immediately whenever there was a content-ful packet from the keyboard (regardless of size), timeout only matters because we also want to probe for changes to config.json decently often. (though, I suppose this could be done on a separate thread, with it's own separate sleep() interval)

I'm under the impression that file reads on python use select.select() under the hood, for threading related reason, or maybe I have that backwards.

Python definitely has select() and poll() (see select.select() and select.poll()) and I believe that select.select() is the predominant blocking call in async python. I'm not sure that we have enough going on that we'd want to restructure just to use it explicitly.

And waiting for changes to config.json isn't a stream read, so I'm not sure there's a way to turn that into a select()able input/event.

[suabo]

I just found that the device has a default timeout value which you can get via libusb.

As in ctrl_transfer, the timeout parameter is optional. When the timeout is omitted, it is used the Device.default_timeout property as the operation timeout.

Found here: https://github.com/pyusb/pyusb/blob/master/docs/tutorial.rst

I already checked the responsiveness of the keys with a higher timeout and it don't matter for the read to finish. On my device the default is set to 1000.

I can't see why the usb timeout is related to the signal send by filesystem?

I also discovered an issue when i wanted to close the process with ctrl+c, the response on that was equivalent to the set usb timeout. I don't really know how signal handling is done in python but it looks like the loop in the main function is blocking all the signals, even terminating ones. If the read method on pyusb would take a callback function as parameter it would reflect something like a poll(). But we just have bEndpointAddress, wMaxPacketSize and iTimeout as parameters.

What do you think about setting the default value from the device?

[hbregalad]

I'm down with that.

[suabo]

= 0.3.0 the default value of the device is used and therefore CPU usage is much less. Please update