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

CP-52709: use timeslices shorter than 50ms #6177

Open
wants to merge 5 commits into
base: feature/perf
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ocaml/libs/timeslice/dune
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
(library
(name xapi_timeslice)
(package xapi)
(package xapi-idl)
(libraries threads.posix mtime mtime.clock.os)
)
1 change: 1 addition & 0 deletions ocaml/xapi-idl/lib/dune
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
unix
uri
uuidm
xapi_timeslice
xapi-backtrace
xapi-consts
xapi-log
Expand Down
25 changes: 25 additions & 0 deletions ocaml/xapi-idl/lib/xcp_service.ml
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,25 @@ let setify =
in
loop []

(** How long to let an OCaml thread run, before
switching to another thread.
This needs to be as small as possible to reduce latency.

Too small values reduce performance due to context switching overheads

4ms = 1/HZ in Dom0 seems like a good default,
a better value will be written by a boot time service.
*)
let timeslice = ref 0.05

let apply_timeslice () =
let interval = !timeslice in
D.debug "Setting timeslice to %.3fs" interval ;
if interval >= 0.05 then
D.debug "Timeslice same as or larger than default: not setting"
Copy link
Member

@robhoes robhoes Dec 13, 2024

Choose a reason for hiding this comment

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

Then where is Xapi_timeslice.Timeslice.set called for the default case?

Copy link
Member

Choose a reason for hiding this comment

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

Oh you mean the OCaml default of 50 ms.

Copy link
Contributor Author

@edwintorok edwintorok Dec 13, 2024

Choose a reason for hiding this comment

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

Yeah the default timeslice is always there, this is in addition to that, it is not possible to turn off the default 50 ms switching.
I should probably document that.

Copy link
Contributor

Choose a reason for hiding this comment

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

So the idea is to add additional yields; would suggest to add __FUNCTION__ to log messages.

else
Xapi_timeslice.Timeslice.set interval

let common_options =
[
( "use-switch"
Expand Down Expand Up @@ -236,6 +255,11 @@ let common_options =
, (fun () -> !config_dir)
, "Location of directory containing configuration file fragments"
)
; ( "timeslice"
, Arg.Set_float timeslice
, (fun () -> Printf.sprintf "%.3f" !timeslice)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure seconds is the best unit here when we mostly talk about milliseconds. However, this has enough resolution down to 1ms.

, "timeslice in seconds"
)
]

let loglevel () = !log_level
Expand Down Expand Up @@ -454,6 +478,7 @@ let configure_common ~options ~resources arg_parse_fn =
failwith (String.concat "\n" lines)
)
resources ;
apply_timeslice () ;
Copy link
Contributor

Choose a reason for hiding this comment

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

Would adjust_timeslice be a better name?

Sys.set_signal Sys.sigpipe Sys.Signal_ignore

let configure ?(argv = Sys.argv) ?(options = []) ?(resources = []) () =
Expand Down