Skip to content

Commit

Permalink
CP-53335, topology: Avoid duplicates in candidate NUMA nodes
Browse files Browse the repository at this point in the history
It's unclear why the candidates with single nodes where always added, since the
algorithm that generates all the subsets already includes these.

Signed-off-by: Pau Ruiz Safont <[email protected]>
  • Loading branch information
psafont committed Feb 3, 2025
1 parent a2ac968 commit d8d35cb
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 27 deletions.
29 changes: 8 additions & 21 deletions ocaml/xenopsd/lib/topology.ml
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,6 @@ let seq_sort ~cmp s =
let a = Array.of_seq s in
Array.fast_sort cmp a ; Array.to_seq a

(** [seq_append a b] is the sequence [a] followed by [b] *)
let seq_append (a : 'a Seq.t) (b : 'a Seq.t) =
let rec next v () =
match v () with Seq.Nil -> b () | Seq.Cons (x, xs) -> Seq.Cons (x, next xs)
in
next a

module NUMA = struct
type node = Node of int

Expand Down Expand Up @@ -194,25 +187,19 @@ module NUMA = struct
None
)
in
let single_nodes =
valid_nodes
|> Seq.map (fun i ->
let self_distance = d.(i).(i) in
(distance_to_candidate self_distance, Seq.return i)
)
in
let numa_nodes = Array.length d in
let nodes =
if numa_nodes > 16 then
(* try just the single nodes, and give up (use all nodes otherwise) to
avoid exponential running time. We could do better here, e.g. by
(* Avoid generating too many candidates because of the exponential
running time. We could do better here, e.g. by
reducing the matrix *)
single_nodes
else
valid_nodes
|> seq_all_subsets
|> Seq.filter_map (node_distances d)
|> seq_append single_nodes
|> Seq.map (fun i ->
let self = d.(i).(i) in
(distance_to_candidate self, Seq.return i)
)
else
valid_nodes |> seq_all_subsets |> Seq.filter_map (node_distances d)
in
nodes
|> seq_sort ~cmp:dist_cmp
Expand Down
9 changes: 3 additions & 6 deletions ocaml/xenopsd/test/test_topology.ml
Original file line number Diff line number Diff line change
Expand Up @@ -281,17 +281,14 @@ let allocate_tests =
let distances_tests =
let specs =
[
( "Last node is unreachable"
, Distances.unreachable_last
, Some [(10., [0]); (10., [0])]
)
("Last node is unreachable", Distances.unreachable_last, Some [(10., [0])])
; ( "Node in the middle is unreachable"
, Distances.unreachable_middle
, Some [(10., [0]); (10., [2]); (10., [0]); (10., [2]); (15., [0; 2])]
, Some [(10., [0]); (10., [2]); (15., [0; 2])]
)
; ( "The first two nodes are unreachable"
, Distances.unreachable_two
, Some [(10., [2]); (10., [2])]
, Some [(10., [2])]
)
; ("All nodes are unreachable", Distances.none_reachable, None)
]
Expand Down

0 comments on commit d8d35cb

Please sign in to comment.