Improve panic messages for NodeCollection - track caller now works

Author: orottier

src/render/graph.rs

@@ -193,8 +193,8 @@ impl Graph {
     }
 
     pub fn add_edge(&mut self, source: (AudioNodeId, usize), dest: (AudioNodeId, usize)) {
-        self.nodes[source.0]
-            .get_mut()
+        self.nodes
+            .get_unchecked_mut(source.0)
             .outgoing_edges
             .push(OutgoingEdge {
                 self_index: source.1,
@@ -206,8 +206,8 @@ impl Graph {
     }
 
     pub fn remove_edge(&mut self, source: AudioNodeId, dest: AudioNodeId) {
-        self.nodes[source]
-            .get_mut()
+        self.nodes
+            .get_unchecked_mut(source)
             .outgoing_edges
             .retain(|edge| edge.other_id != dest);
 
@@ -216,7 +216,7 @@ impl Graph {
 
     pub fn remove_edges_from(&mut self, source: AudioNodeId) {
         // Remove outgoing edges
-        self.nodes[source].get_mut().outgoing_edges.clear();
+        self.nodes.get_unchecked_mut(source).outgoing_edges.clear();
 
         // Remove incoming edges - we need to traverse all nodes
         self.nodes.values_mut().for_each(|node| {
@@ -241,21 +241,27 @@ impl Graph {
     }
 
     pub fn mark_cycle_breaker(&mut self, index: AudioNodeId) {
-        self.nodes[index].get_mut().cycle_breaker = true;
+        self.nodes.get_unchecked_mut(index).cycle_breaker = true;
     }
 
     pub fn set_channel_count(&mut self, index: AudioNodeId, v: usize) {
-        self.nodes[index].get_mut().channel_config.count = v;
+        self.nodes.get_unchecked_mut(index).channel_config.count = v;
     }
     pub fn set_channel_count_mode(&mut self, index: AudioNodeId, v: ChannelCountMode) {
-        self.nodes[index].get_mut().channel_config.count_mode = v;
+        self.nodes
+            .get_unchecked_mut(index)
+            .channel_config
+            .count_mode = v;
     }
     pub fn set_channel_interpretation(&mut self, index: AudioNodeId, v: ChannelInterpretation) {
-        self.nodes[index].get_mut().channel_config.interpretation = v;
+        self.nodes
+            .get_unchecked_mut(index)
+            .channel_config
+            .interpretation = v;
     }
 
     pub fn route_message(&mut self, index: AudioNodeId, msg: &mut dyn Any) {
-        self.nodes[index].get_mut().processor.onmessage(msg);
+        self.nodes.get_unchecked_mut(index).processor.onmessage(msg);
     }
 
     /// Helper function for `order_nodes` - traverse node and outgoing edges
@@ -278,7 +284,7 @@ impl Graph {
             let cycle_breaker_node = marked_temp
                 .iter()
                 .skip(pos)
-                .find(|&&node_id| self.nodes[node_id].borrow().cycle_breaker);
+                .find(|&&node_id| self.nodes.get_unchecked(node_id).borrow().cycle_breaker);
 
             match cycle_breaker_node {
                 Some(&node_id) => {
@@ -307,7 +313,13 @@ impl Graph {
         marked_temp.push(node_id);
 
         // Visit outgoing nodes, and call `visit` on them recursively
-        for edge in self.nodes[node_id].borrow().outgoing_edges.iter() {
+        for edge in self
+            .nodes
+            .get_unchecked(node_id)
+            .borrow()
+            .outgoing_edges
+            .iter()
+        {
             let cycle_breaker_applied = self.visit(
                 edge.other_id,
                 marked,
@@ -386,9 +398,11 @@ impl Graph {
 
             if cycle_breaker_applied {
                 // clear the outgoing edges of the nodes that have been recognized as cycle breaker
-                let nodes = &mut self.nodes;
                 cycle_breakers.iter().for_each(|node_id| {
-                    nodes[*node_id].get_mut().outgoing_edges.clear();
+                    self.nodes
+                        .get_unchecked_mut(*node_id)
+                        .outgoing_edges
+                        .clear();
                 });
 
                 continue;
@@ -423,16 +437,13 @@ impl Graph {
         // keep track of end-of-lifecyle nodes
         let mut nodes_dropped = false;
 
-        // for borrow-checker reasons, move mutable borrow of nodes out of self
-        let nodes = &mut self.nodes;
-
         // process every node, in topological sorted order
         self.ordered.iter().for_each(|index| {
             // acquire a mutable borrow of the current processing node
-            let mut node = nodes[*index].borrow_mut();
+            let mut node = self.nodes.get_unchecked(*index).borrow_mut();
 
             // let the current node process (catch any panics that may occur)
-            let params = AudioParamValues::from(nodes);
+            let params = AudioParamValues::from(&self.nodes);
             scope.node_id.set(*index);
             let (success, tail_time) = {
                 // We are abusing AssertUnwindSafe here, we cannot guarantee it upholds.
@@ -456,7 +467,7 @@ impl Graph {
                 // audio params are connected to the 'hidden' usize::MAX output, ignore them here
                 .filter(|edge| edge.other_index != usize::MAX)
                 .for_each(|edge| {
-                    let mut output_node = nodes[edge.other_id].borrow_mut();
+                    let mut output_node = self.nodes.get_unchecked(edge.other_id).borrow_mut();
                     output_node.has_inputs_connected = true;
                     let signal = &node.outputs[edge.self_index];
                     let channel_config = &output_node.channel_config.clone();
@@ -482,7 +493,7 @@ impl Graph {
             // Check if we can decommission this node (end of life)
             if can_free {
                 // Node is dropped, remove it from the node list
-                let mut node = nodes.remove(*index).into_inner();
+                let mut node = self.nodes.remove(*index).into_inner();
                 self.reclaim_id_channel
                     .push(node.reclaim_id.take().unwrap());
                 drop(node);
@@ -492,7 +503,7 @@ impl Graph {
 
                 // Nodes are only dropped when they do not have incoming connections.
                 // But they may have AudioParams feeding into them, these can de dropped too.
-                nodes.retain(|id, node| {
+                self.nodes.retain(|id, node| {
                     let node = node.get_mut(); // unwrap the RefCell
 
                     // Check if this node was connected to the dropped node. In that case, it is
@@ -524,7 +535,7 @@ impl Graph {
         if nodes_dropped {
             let mut i = 0;
             while i < self.ordered.len() {
-                if nodes.get(self.ordered[i]).is_none() {
+                if !self.nodes.contains(self.ordered[i]) {
                     self.ordered.remove(i);
                 } else {
                     i += 1;
@@ -533,7 +544,7 @@ impl Graph {
         }
 
         // Return the output buffer of destination node
-        &self.nodes[AudioNodeId(0)].get_mut().outputs[0]
+        &self.nodes.get_unchecked_mut(AudioNodeId(0)).outputs[0]
     }
 }
 
@@ -735,7 +746,10 @@ mod tests {
         // dropped and the AudioNodeId(3) should be reclaimed
         add_node(&mut graph, 2, node.clone());
         // Mark the node as 'detached from the control thread', so it is allowed to drop
-        graph.nodes[AudioNodeId(2)].get_mut().control_handle_dropped = true;
+        graph
+            .nodes
+            .get_unchecked_mut(AudioNodeId(2))
+            .control_handle_dropped = true;
 
         // Connect the regular node to the AudioDestinationNode
         add_edge(&mut graph, 2, 0);
@@ -777,7 +791,10 @@ mod tests {
         // dropped and the AudioNodeId(3) should be reclaimed
         add_node(&mut graph, 2, node.clone());
         // Mark the node as 'detached from the control thread', so it is allowed to drop
-        graph.nodes[AudioNodeId(2)].get_mut().control_handle_dropped = true;
+        graph
+            .nodes
+            .get_unchecked_mut(AudioNodeId(2))
+            .control_handle_dropped = true;
 
         // Connect the regular node to the AudioDestinationNode
         add_edge(&mut graph, 2, 0);
@@ -786,7 +803,10 @@ mod tests {
         let param = Box::new(TestNode { tail_time: true }); // audio params have tail time true
         add_node(&mut graph, 3, param);
         // Mark the node as 'detached from the control thread', so it is allowed to drop
-        graph.nodes[AudioNodeId(3)].get_mut().control_handle_dropped = true;
+        graph
+            .nodes
+            .get_unchecked_mut(AudioNodeId(3))
+            .control_handle_dropped = true;
 
         // Connect the audioparam to the regular node
         add_audioparam(&mut graph, 3, 2);
@@ -828,7 +848,10 @@ mod tests {
         add_node(&mut graph, 2, node);
 
         // Mark the node as 'detached from the control thread', so it is allowed to drop
-        graph.nodes[AudioNodeId(2)].get_mut().control_handle_dropped = true;
+        graph
+            .nodes
+            .get_unchecked_mut(AudioNodeId(2))
+            .control_handle_dropped = true;
 
         // Render a single quantum
         let scope = AudioWorkletGlobalScope {

src/render/node_collection.rs

@@ -2,7 +2,6 @@ use crate::context::{AudioNodeId, DESTINATION_NODE_ID};
 use crate::render::graph::Node;
 
 use std::cell::RefCell;
-use std::ops::{Index, IndexMut};
 
 #[derive(Debug)]
 pub(crate) struct NodeCollection {
@@ -59,8 +58,8 @@ impl NodeCollection {
     }
 
     #[inline(always)]
-    pub fn get(&self, index: AudioNodeId) -> Option<&RefCell<Node>> {
-        self.nodes[index.0 as usize].as_ref()
+    pub fn contains(&self, index: AudioNodeId) -> bool {
+        self.nodes[index.0 as usize].is_some()
     }
 
     #[inline(always)]
@@ -81,31 +80,17 @@ impl NodeCollection {
             }
         })
     }
-}
-
-impl Index<AudioNodeId> for NodeCollection {
-    type Output = RefCell<Node>;
 
     #[track_caller]
     #[inline(always)]
-    fn index(&self, index: AudioNodeId) -> &Self::Output {
-        self.nodes
-            .get(index.0 as usize)
-            .unwrap_or_else(|| panic!("Unexpected index {} for NodeCollection", index.0))
-            .as_ref()
-            .unwrap_or_else(|| panic!("Index {} for dropped Node in NodeCollection", index.0))
+    pub fn get_unchecked(&self, index: AudioNodeId) -> &RefCell<Node> {
+        self.nodes[index.0 as usize].as_ref().unwrap()
     }
-}
 
-impl IndexMut<AudioNodeId> for NodeCollection {
     #[track_caller]
     #[inline(always)]
-    fn index_mut(&mut self, index: AudioNodeId) -> &mut Self::Output {
-        self.nodes
-            .get_mut(index.0 as usize)
-            .unwrap_or_else(|| panic!("Unexpected index {} for NodeCollection", index.0))
-            .as_mut()
-            .unwrap_or_else(|| panic!("Index {} for dropped Node in NodeCollection", index.0))
+    pub fn get_unchecked_mut(&mut self, index: AudioNodeId) -> &mut Node {
+        self.nodes[index.0 as usize].as_mut().unwrap().get_mut()
     }
 }
 

src/render/processor.rs

@@ -195,7 +195,7 @@ impl<'a> AudioParamValues<'a> {
     /// provide a slice of length equal to the render quantum size (default: 128)
     #[allow(clippy::missing_panics_doc)]
     pub fn get(&self, index: &AudioParamId) -> impl Deref<Target = [f32]> + '_ {
-        DerefAudioRenderQuantumChannel(self.nodes[index.into()].borrow())
+        DerefAudioRenderQuantumChannel(self.nodes.get_unchecked(index.into()).borrow())
     }
 
     pub(crate) fn listener_params(&self) -> [impl Deref<Target = [f32]> + '_; 9] {