hawkw/mycelium

maitake: scheduler trait is kind of weird

Opened this issue · 2 comments

hawkw commented

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 commented

cc @jamesmunns any opinions?

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:

  • Make it a sealed trait (I did this in #251, mnemos still builds with this change)
  • Remove the trait entirely

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.