Skip to content

Commit b945e10

Browse files
committed
Optimize fastpath in Pool_role
Use an Atomic that can be accessed without holding the mutex. On the fastpath, where the role is already set this returns much more quickly. On the slowpath we still acquire the mutex and update the value as we did before, although we read the atomic value again to check whether we've raced with another thread that has updated it meanwhile already. 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 } ``` Signed-off-by: Edwin Török <[email protected]>
1 parent 398719e commit b945e10

File tree

2 files changed

+17
-18
lines changed

2 files changed

+17
-18
lines changed

ocaml/tests/bench/bench_cached_reads.ml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ let run () =
77
let mutex_workload =
88
Bechamel_simple_cli.thread_workload ~before:ignore ~after:ignore ~run
99

10-
let free_threads (stop, t) = Atomic.set stop true ; Array.iter Thread.join t
11-
1210
let benchmarks =
1311
Test.make_grouped ~name:"Cached reads"
1412
[Test.make ~name:"Pool_role.is_master" (Staged.stage Pool_role.is_master)]

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)