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

Blocking processes / scheduler #80

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
Draft

Conversation

omnisci3nce
Copy link

Addresses #67 (a first pass at least)

Comment on lines +511 to +514
(* Override the handle exit function *)
(* let handle_exit_blocking_proc pool sch proc reason = *)
(* Scheduler.handle_exit_proc pool sch.scheduler proc reason; *)
(* remove_from_pool pool sch *)
Copy link
Author

@omnisci3nce omnisci3nce May 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking I could somehow have a custom handle_exit function but just having the domain doesnt mean I can shutdown and remove the scheduler I think, because I really need to close it from inside the run + run_loop scheduler loop, therefore I commented out this code here.

Would this still work if I had a way of shutting down a scheduler from outside that loop? Presumably this handle_exit_blocking is running from inside the Domain so could I just raise an exception here instead of the bool flags approach?


let pp_flag fmt t =
match t with
| Trap_exit b -> Format.fprintf fmt "trap_exit <- %b" b
| Priority p -> Format.fprintf fmt "priority <- %s" (P.priority_to_string p)
| _ -> failwith "TODO"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will fix this once we're happy with the other code


Exception exn)
in
Process.set_flag proc (IsBlockingProc true);
Copy link
Author

@omnisci3nce omnisci3nce May 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By adding the IsBlockingProc flag here we're basically saying "when this processes exits, we can go ahead and shutdown the scheduler".
The main thought process here is I don't want to modify the raw Process and Scheduler code too much making a bunch of duplicated functions; I'd rather share as much of the code that currently exists.

I struggled to figure out how to do this shutdown of a blocking scheduler when the process finishes from the scheduler's point of view rather than the process's, as there's no way to distinguish a Blocking_scheduler.t from a Scheduler.t in the run and run_loop code for the reason above - wanting to keep all the code accepting Scheduler.t the same without duplicating it. Hence the mutable stop : bool

@@ -26,12 +27,15 @@ type io = {
mutable calls_send : int;
}

type blocking = { scheduler : t; domain : unit Domain.t }
Copy link
Author

@omnisci3nce omnisci3nce May 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially we don't need this type but when you said

new Blocking_scheduler that wraps a Scheduler and adds logic for removing the domains from the blocking_processes domain list

I'm not 100% sure in OCaml what the best way of having a "wrapping" type is without having say a Scheduler trait like in Rust i.e. adhoc polymorphism. Is this type + the Blocking_scheduler a good way of doing this do you think?

(I'm actually not sure we technically need to keep a list. Does the domain live as long as its function body i.e. as long as the Scheduler's loop? If we shut the scheduler down the Domain should be closed/freed/whatever you call it automatically if that were the case I think. But it's probably still nice to have a typed record for this and keep that data around in case we need to add to it later)

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.

None yet

1 participant