oak_runtime: Track Nodes with u64 NodeRefs #690
Conversation
| #[derive(Debug)] | ||
| struct Node { | ||
| reference: NodeRef, | ||
| join_handle: Option<JoinHandle>, |
There was a problem hiding this comment.
I guess this may become HashSet<JoinHandle> in the future, if / when we support multiple threads per node?
There was a problem hiding this comment.
Yeah, this can be expanded
| let mut node_threads = self.node_threads.lock().unwrap(); | ||
| { | ||
| let nodes = self.nodes.lock().unwrap(); | ||
| debug!("nodes: {:?}", nodes); |
There was a problem hiding this comment.
Is this intended? Why not just move this after line 99?
There was a problem hiding this comment.
Removed: left over from checking things were working as intended :)
| @@ -73,6 +72,7 @@ impl Configuration { | |||
| &self, | |||
| config_name: &str, // Used for pretty debugging | |||
| runtime: RuntimeRef, | |||
| _noderef: NodeRef, | |||
There was a problem hiding this comment.
When a Node is created and the runtime generates a new reference/id, i pass it here into the newly created threads so it knows its own reference. Currently unused as the runtime/channel methods don't use the references.
| fn new_node_handle(&self) -> NodeRef { | ||
| let mut handle; | ||
|
|
||
| loop { |
There was a problem hiding this comment.
I feel it would be sufficient / simpler to just use an incrementing (atomic) counter, similar to how Process IDs work in OSs.
There was a problem hiding this comment.
Done. Argh, I had randomized it as I think thats the direct you want to go with Channels.
There was a problem hiding this comment.
I think channels will also not be randomized, but channel handles (read or write) will be, since they are the ones used as capabilities.
| @@ -47,7 +56,8 @@ pub struct Runtime { | |||
| configurations: HashMap<String, node::Configuration>, | |||
| terminating: AtomicBool, | |||
| channels: Mutex<Channels>, | |||
| node_threads: Mutex<Vec<JoinHandle>>, | |||
| nodes: Mutex<HashMap<NodeRef, Node>>, | |||
| next_node: AtomicU64, | |||
There was a problem hiding this comment.
(optional)
| next_node: AtomicU64, | |
| next_node_reference: AtomicU64, |
| @@ -312,20 +323,36 @@ impl RuntimeRef { | |||
| return Err(OakStatus::ErrTerminated); | |||
| } | |||
|
|
|||
| let mut node_threads = self.node_threads.lock().unwrap(); | |||
| let reference = NodeRef(self.next_node.fetch_add(1, SeqCst)); | |||
There was a problem hiding this comment.
(optional) Could have a method next_node_reference or similar on self.
There was a problem hiding this comment.
Done new_node_reference
| Ok(join_handle) => { | ||
| nodes.insert( | ||
| reference, | ||
| Node { |
There was a problem hiding this comment.
Maybe Configuration::new_instance should return a Node struct directly?
There was a problem hiding this comment.
Currently the implementation of Node is private to the runtime, and would require being visible to the crate for this. I think the encapsulation to only the runtime is preferable, but we can change it in future PRs as necessary.
| fn new_node_handle(&self) -> NodeRef { | ||
| let mut handle; | ||
|
|
||
| loop { |
There was a problem hiding this comment.
I think channels will also not be randomized, but channel handles (read or write) will be, since they are the ones used as capabilities.
Checklist
cover any TODOs and/or unfinished work.
construction.