Skip to content

Commit 71f64c8

Browse files
authored
Optimize Pool_role fastpath (#6279)
Before (measuring both unloaded, and loaded situation where other threads are accessing the mutex): ``` Running benchmarks (no workloads) ╭────────────────────────────────────┬───────────────────────────┬───────────────────────────┬───────────────────────────╮ │name │ major-allocated │ minor-allocated │ monotonic-clock │ ├────────────────────────────────────┼───────────────────────────┼───────────────────────────┼───────────────────────────┤ │ Cached reads/Pool_role.is_master │ 0.0000 mjw/run│ 4.0000 mnw/run│ 23.6039 ns/run│ ╰────────────────────────────────────┴───────────────────────────┴───────────────────────────┴───────────────────────────╯ Cached reads/Pool_role.is_master (ns): { monotonic-clock per run = 23.603889 (confidence: 23.644693 to 23.568067); r² = Some 0.999899 } Running benchmarks (workloads) ╭────────────────────────────────────┬───────────────────────────┬───────────────────────────┬───────────────────────────╮ │name │ major-allocated │ minor-allocated │ monotonic-clock │ ├────────────────────────────────────┼───────────────────────────┼───────────────────────────┼───────────────────────────┤ │ Cached reads/Pool_role.is_master │ 0.0000 mjw/run│ 7.4201 mnw/run│ 76.4067 ns/run│ ╰────────────────────────────────────┴───────────────────────────┴───────────────────────────┴───────────────────────────╯ Cached reads/Pool_role.is_master (ns): { monotonic-clock per run = 76.406674 (confidence: 86.718075 to 66.336404); r² = Some 0.608131 } ``` After: ``` Running benchmarks (no workloads) ╭────────────────────────────────────┬───────────────────────────┬───────────────────────────┬───────────────────────────╮ │name │ major-allocated │ minor-allocated │ monotonic-clock │ ├────────────────────────────────────┼───────────────────────────┼───────────────────────────┼───────────────────────────┤ │ Cached reads/Pool_role.is_master │ 0.0000 mjw/run│ 0.0000 mnw/run│ 3.1256 ns/run│ ╰────────────────────────────────────┴───────────────────────────┴───────────────────────────┴───────────────────────────╯ Cached reads/Pool_role.is_master (ns): { monotonic-clock per run = 3.125616 (confidence: 3.131412 to 3.120373); r² = Some 0.999885 } Running benchmarks (workloads) ╭────────────────────────────────────┬───────────────────────────┬───────────────────────────┬───────────────────────────╮ │name │ major-allocated │ minor-allocated │ monotonic-clock │ ├────────────────────────────────────┼───────────────────────────┼───────────────────────────┼───────────────────────────┤ │ Cached reads/Pool_role.is_master │ 0.0000 mjw/run│ 0.0000 mnw/run│ 6.2872 ns/run│ ╰────────────────────────────────────┴───────────────────────────┴───────────────────────────┴───────────────────────────╯ Cached reads/Pool_role.is_master (ns): { monotonic-clock per run = 6.287197 (confidence: 6.737363 to 5.830140); r² = Some 0.706843 } ```
2 parents 51897aa + b945e10 commit 71f64c8

File tree

3 files changed

+32
-17
lines changed

3 files changed

+32
-17
lines changed
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
open Bechamel
2+
3+
let run () =
4+
let _ : bool = Sys.opaque_identity (Pool_role.is_master ()) in
5+
()
6+
7+
let mutex_workload =
8+
Bechamel_simple_cli.thread_workload ~before:ignore ~after:ignore ~run
9+
10+
let benchmarks =
11+
Test.make_grouped ~name:"Cached reads"
12+
[Test.make ~name:"Pool_role.is_master" (Staged.stage Pool_role.is_master)]
13+
14+
let () = Bechamel_simple_cli.cli ~workloads:[mutex_workload] benchmarks

ocaml/tests/bench/dune

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
(executables
2-
(names bench_tracing bench_uuid bench_throttle2)
2+
(names bench_tracing bench_uuid bench_throttle2 bench_cached_reads)
33
(libraries tracing bechamel bechamel-notty notty.unix tracing_export threads.posix fmt notty uuid xapi_aux tests_common log xapi_internal)
44
)

ocaml/xapi-aux/pool_role.ml

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -28,21 +28,19 @@ type t =
2828
(* IP address *)
2929
| Broken
3030

31-
let role = ref None
31+
let role = Atomic.make None
3232

33-
let role_unit_tests = ref false
33+
let role_unit_tests = Atomic.make false
3434

3535
let role_m = Mutex.create ()
3636

3737
let with_pool_role_lock f = Xapi_stdext_threads.Threadext.Mutex.execute role_m f
3838

3939
let set_pool_role_for_test () =
40-
with_pool_role_lock (fun _ ->
41-
role := Some Master ;
42-
role_unit_tests := true
43-
)
40+
Atomic.set role (Some Master) ;
41+
Atomic.set role_unit_tests true
4442

45-
let is_unit_test () = with_pool_role_lock (fun _ -> !role_unit_tests)
43+
let is_unit_test () = Atomic.get role_unit_tests
4644

4745
let string_of = function
4846
| Master ->
@@ -80,15 +78,18 @@ let read_pool_role () =
8078
)
8179

8280
let get_role () =
83-
with_pool_role_lock (fun _ ->
84-
match !role with
85-
| Some x ->
86-
x
87-
| None ->
88-
let r = read_pool_role () in
89-
role := Some r ;
90-
r
91-
)
81+
match Atomic.get role with
82+
| Some x ->
83+
x
84+
| None ->
85+
with_pool_role_lock (fun _ ->
86+
match Atomic.get role with
87+
| Some x ->
88+
x
89+
| None ->
90+
let r = read_pool_role () in
91+
Atomic.set role (Some r) ; r
92+
)
9293

9394
let is_master () = get_role () = Master
9495

0 commit comments

Comments
 (0)