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

Single-Threaded Service #592

Draft
wants to merge 15 commits into
base: beta
Choose a base branch
from

Conversation

jonasBoss
Copy link
Collaborator

This runs the Service in a single process.
It should reduce overhead (only one python interpreter running) and it allows for better Profiling. In addition it allows us to simplify the injector, shared-dict and test-fixtures because we can remove all the communication stuff.

There is one issue with this: the use of a different dbus library dbus-next. This is needed because the current library pydbus needs a GLib.MainLoop, which is incompatible with asyncio. There is a merge-request to add a asyncio compatible GLib.MainLoop, but who knows when this is ready.

Unfortunately dbus-next is not part of the Ubuntu repos.
An alternative could be dbussy but that deliberately introduces inconsistency with method return types (see this issue).
There is also jeepney, but that is missing a high level interface, so it will require extra work.
Both of these alternatives are part of the ubuntu repos. But I still think that dbus-next offers the best interface with only one issue for which I found a workaround.

So the question is:

  • do we want this?
    • I think yes, it will make the code simpler to reason about.
  • Is a single process fast enough to handle multiple devices with high polling rates?
    • this needs testing
  • do we want to, and can we package InputRemapper with dbus-next?

@sezanzeb
Copy link
Owner

sezanzeb commented Jan 10, 2023

can we package InputRemapper with dbus-next?

This is a question for @skitt :)

Can you please tell us if this would be problematic for the ubuntu package?

Is a single process fast enough to handle multiple devices with high polling rates?

Multithreading in python is a bit special apparently. GIL prevents the usage of more than one core. So while multithreading helps to make things concurrent when threads are waiting for i/o, it does not improve performance.

I wonder what the benefits of multithreading over asyncio are.

@sezanzeb
Copy link
Owner

sezanzeb commented Jan 10, 2023

I think it would make sense to release 2.0 before we merge this

@sezanzeb
Copy link
Owner

It should reduce overhead (only one python interpreter running) and it allows for better Profiling. In addition it allows us to simplify the injector, shared-dict and test-fixtures because we can remove all the communication stuff.

Sounds like good benefits

@jonasBoss
Copy link
Collaborator Author

jonasBoss commented Jan 10, 2023

I think it would make sense to release 2.0 before we merge this

I think so as well

I wonder what the benefits of multithreading over asyncio are.

From what I know there are no benefits to multithreading over asyncio.
The only benefit is that it is easy to implement. But only if the Threads don't need synchronization. If they do, thing get complicated with locks and stuff.
Since InputRemapper is using multiprocessing, we could switch to multithreading without any issues, and still get some simplification because we could replace all the communication pipes with queues.

With mutlithreading we don't know where the execution is interrupted and another thread runs. So it is involuntary or implicit concurrency.

With aio we know that other coroutines can only run when the execution is awaiting something. So it is voluntary or explicit concurrency.

@skitt
Copy link
Contributor

skitt commented Jan 10, 2023

From a packaging perspective dbus-next will be available in Debian 12 and is available in Ubuntu 22.10 (imported from Debian). So the distro packages will be OK, but the package provided on GitHub will be usable on a limited selection of releases.

@sezanzeb
Copy link
Owner

Thank you!

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

Successfully merging this pull request may close these issues.

3 participants