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

Rewrite thread-local actor waking mechanism #409

Open
Thomasdezeeuw opened this issue Mar 2, 2021 · 0 comments
Open

Rewrite thread-local actor waking mechanism #409

Thomasdezeeuw opened this issue Mar 2, 2021 · 0 comments
Labels
performance Performance related issue. priority:low Low priority issue.

Comments

@Thomasdezeeuw
Copy link
Owner

Thomasdezeeuw commented Mar 2, 2021

The task::Waker implementation for the thread-local actors is currently based on sending it's process id into an unbounded channel. The worker thread reads from this channel and schedules the actors accordingly. The current implementation is located here: https://github.com/Thomasdezeeuw/heph/blob/40984a97301706e32e7237a1535a7d06f5e157e7/src/rt/waker.rs.

This has two problems:

  1. The channel is unbounded which means it could use a lot of memory if the worker threads is blocked, e.g. by a long running actor.
  2. It requires the involvement of the worker thread to schedule the actor, it would be nice if this could be done directly.

Towards a new implementation

Requirements:

  • Accessible from any thread. We don't control from which thread the task::Waker implementation is called.
  • The worker thread that owns the scheduler in which the to-be-awoken actor runs must not be blocked. I.e. removing a process from the ready queue mustn't block.
  • Would be nice if we didn't do a large amount of work if the process is already marked as ready-to-run, or was stopped.

Possible implementations

I thought of two possible implementations:

  1. A concurrent hashset. The waker implementation would add the process id to the set when waking, the worker thread would then drain the set and schedule all actors based on the process ids. Benefit over the current implementation is that it's not unbounded, but not much more.
  2. Directly schedule the actor in the thread-local scheduler. We already do this for the thread-safe scheduler. The downside of this is that we lose the benefit over having a thread-local scheduler to begin with.
@Thomasdezeeuw Thomasdezeeuw added the performance Performance related issue. label Mar 2, 2021
@Thomasdezeeuw Thomasdezeeuw added this to the First release milestone Mar 2, 2021
@Thomasdezeeuw Thomasdezeeuw added the priority:low Low priority issue. label Mar 12, 2021
@Thomasdezeeuw Thomasdezeeuw removed this from the First release milestone Apr 1, 2021
@Thomasdezeeuw Thomasdezeeuw mentioned this issue Apr 21, 2023
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance related issue. priority:low Low priority issue.
Projects
None yet
Development

No branches or pull requests

1 participant