-
Notifications
You must be signed in to change notification settings - Fork 293
Update VM's numa state #6747
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
base: feature/numa9
Are you sure you want to change the base?
Update VM's numa state #6747
Conversation
Signed-off-by: Christian Lindig <[email protected]>
| ; numa_nodes= numa.Xenctrlext.DomainNuma.nodes | ||
| ; numa_node_memory= | ||
| Array.fold_left | ||
| (fun (i, assoc) mem -> (i + 1, (i, mem) :: assoc)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would Array.mapi simplify this code? (followed by Array.to_list)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Let's mapi do the work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using Array.mapi means a new array is allocated, do we want that?
One way to avoid it would be:
numa.Xenctrlext.DomainNuma.memory
|> Array.to_seq
|> Seq.mapi (fun i mem -> (i, mem))
|> List.of_seqThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this cheaper? I don't understand the cost of Seq well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seq is a lazy structure, and the transformations only get executed for building the end result, so instead of creating a new array and a new list, this should create a new list only (and the closures for doing the transformation for each value)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I expect the intermediary closures will end up using more space than creating a fresh array: the List.of_seq amounts to consecutively forcing the sequence (applying the closures that arise from the partial application of the next array index). The previous closures will probably be around until the next minor sweep.
I'm actually really fond of the original version of the code. It avoids the space problem entirely.
Signed-off-by: Christian Lindig <[email protected]>
The missing bits: when xenopsd reports a new VM state; update its numa state in xapi accordingly.