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

maitake: scheduler trait is kind of weird #238

Open
hawkw opened this issue Jun 23, 2022 · 2 comments
Open

maitake: scheduler trait is kind of weird #238

hawkw opened this issue Jun 23, 2022 · 2 comments
Labels
crate/maitake Related to the `maitake` crate

Comments

@hawkw
Copy link
Owner

hawkw commented Jun 23, 2022

Currently, maitake has a Schedule trait that abstracts over a scheduler that can schedule a task:

pub trait Schedule: Sized + Clone {

This has to be a public trait, as it's one of the trait bounds on Task's generics. However, the trait is kind of weird if user code intends to implement it. The only way to schedule a task is to push it to the scheduler's run queue, which is a cordyceps::MpscQueue. However, the Linked<mpsc_queue::Links<...>> impl for task::Header is private, so downstream code cannot use this to implement Schedule for its own types.

There are some questions we may eventually want to work out:

  1. do we want Schedule to be an extension point? is there a reason for user code to implement Schedule for its own types?
  2. if we want user code to implement this trait, should we consider changing it from having a schedule method to having a run_queue accessor or similar? that way, the Task code can handle actually pushing itself to the scheduler run queue, and it just asks that a scheduler own a run queue.
  3. if we don't want user code to implement Schedule, should we
  • remove the trait and replace it with some kind of type erased SchedulerRef type (potentially adding dyn dispatch overhead...)?
  • make the trait impossible to implement in user code explicitly?
  • just document that this isn't intended to be implemented downstream?
@hawkw
Copy link
Owner Author

hawkw commented Jun 23, 2022

cc @jamesmunns any opinions?

@jamesmunns
Copy link
Collaborator

So, "as a downstream user", I don't know if I'd necessarily want to be able to abstract over the scheduler that I'm working with? But if implementing the trait requires intimate knowledge of the scheduler implementation like you've said above, I'd probably say it's better to:

It's acceptable to make it a sealed trait, as the user MUST choose one of the Schedule impls provided by Maitake, and can use them to fulfill the S: Schedule trait bound, though they can't use that to make THIER downstream types generic over some S in the future (because they can't name the Schedule trait).

This seems fine to me? But again, I'm not trying to be generic over my scheduler implementations. If you can think of a reason that someone would want to do that, you could close #251, but if not, I'd say merge it, or just yeet the trait entirely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate/maitake Related to the `maitake` crate
Projects
None yet
Development

No branches or pull requests

2 participants