Merge pull request #379 from orottier/feature/store-nodes-in-vec-not-hashmap

Experiment: store nodes in contiguous Vec instead of HashMap

Author: orottier

Cargo.toml

@@ -29,7 +29,6 @@ no_denormals = "0.1.2"
 num-complex = "0.4"
 realfft = "3.3"
 rubato = "0.14"
-rustc-hash = "1.1"
 smallvec = "1.11"
 symphonia = { version = "0.5", default-features = false }
 vecmath = "1.0"

src/context/concrete_base.rs

@@ -17,6 +17,34 @@ use crossbeam_channel::{Receiver, SendError, Sender};
 use std::sync::atomic::{AtomicU64, AtomicU8, Ordering};
 use std::sync::{Arc, Mutex, RwLock, RwLockWriteGuard};
 
+/// This struct assigns new [`AudioNodeId`]s for [`AudioNode`]s
+///
+/// It reuses the ids of decommissioned nodes to prevent unbounded growth of the audio graphs node
+/// list (which is stored in a Vec indexed by the AudioNodeId).
+struct AudioNodeIdProvider {
+    /// incrementing id
+    id_inc: AtomicU64,
+    /// receiver for decommissioned AudioNodeIds, which can be reused
+    id_consumer: Mutex<llq::Consumer<AudioNodeId>>,
+}
+
+impl AudioNodeIdProvider {
+    fn new(id_consumer: llq::Consumer<AudioNodeId>) -> Self {
+        Self {
+            id_inc: AtomicU64::new(0),
+            id_consumer: Mutex::new(id_consumer),
+        }
+    }
+
+    fn get(&self) -> AudioNodeId {
+        if let Some(available_id) = self.id_consumer.lock().unwrap().pop() {
+            llq::Node::into_inner(available_id)
+        } else {
+            AudioNodeId(self.id_inc.fetch_add(1, Ordering::Relaxed))
+        }
+    }
+}
+
 /// The struct that corresponds to the Javascript `BaseAudioContext` object.
 ///
 /// This object is returned from the `base()` method on
@@ -47,8 +75,8 @@ struct ConcreteBaseAudioContextInner {
     sample_rate: f32,
     /// max number of speaker output channels
     max_channel_count: usize,
-    /// incrementing id to assign to audio nodes
-    node_id_inc: AtomicU64,
+    /// provider for new AudioNodeIds
+    audio_node_id_provider: AudioNodeIdProvider,
     /// destination node's current channel count
     destination_channel_config: ChannelConfig,
     /// message channel from control to render thread
@@ -83,9 +111,8 @@ impl BaseAudioContext for ConcreteBaseAudioContext {
         &self,
         f: F,
     ) -> T {
-        // create unique identifier for this node
-        let id = self.inner.node_id_inc.fetch_add(1, Ordering::SeqCst);
-        let id = AudioNodeId(id);
+        // create a unique id for this node
+        let id = self.inner.audio_node_id_provider.get();
         let registration = AudioContextRegistration {
             id,
             context: self.clone(),
@@ -97,6 +124,7 @@ impl BaseAudioContext for ConcreteBaseAudioContext {
         // pass the renderer to the audio graph
         let message = ControlMessage::RegisterNode {
             id,
+            reclaim_id: llq::Node::new(id),
             node: render,
             inputs: node.number_of_inputs(),
             outputs: node.number_of_outputs(),
@@ -126,19 +154,22 @@ impl ConcreteBaseAudioContext {
         render_channel: Sender<ControlMessage>,
         event_channel: Option<(Sender<EventDispatch>, Receiver<EventDispatch>)>,
         offline: bool,
+        node_id_consumer: llq::Consumer<AudioNodeId>,
     ) -> Self {
         let event_loop = EventLoop::new();
         let (event_send, event_recv) = match event_channel {
             None => (None, None),
             Some((send, recv)) => (Some(send), Some(recv)),
         };
 
+        let audio_node_id_provider = AudioNodeIdProvider::new(node_id_consumer);
+
         let base_inner = ConcreteBaseAudioContextInner {
             sample_rate,
             max_channel_count,
             render_channel: RwLock::new(render_channel),
             queued_messages: Mutex::new(Vec::new()),
-            node_id_inc: AtomicU64::new(0),
+            audio_node_id_provider,
             destination_channel_config: ChannelConfigOptions::default().into(),
             frames_played,
             queued_audio_listener_msgs: Mutex::new(Vec::new()),
@@ -202,7 +233,10 @@ impl ConcreteBaseAudioContext {
 
         // Validate if the hardcoded node IDs line up
         debug_assert_eq!(
-            base.inner.node_id_inc.load(Ordering::Relaxed),
+            base.inner
+                .audio_node_id_provider
+                .id_inc
+                .load(Ordering::Relaxed),
             LISTENER_PARAM_IDS.end,
         );
 
@@ -420,3 +454,19 @@ impl ConcreteBaseAudioContext {
         self.inner.event_loop.clear_handler(event);
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_provide_node_id() {
+        let (mut id_producer, id_consumer) = llq::Queue::new().split();
+        let provider = AudioNodeIdProvider::new(id_consumer);
+        assert_eq!(provider.get().0, 0); // newly assigned
+        assert_eq!(provider.get().0, 1); // newly assigned
+        id_producer.push(llq::Node::new(AudioNodeId(0)));
+        assert_eq!(provider.get().0, 0); // reused
+        assert_eq!(provider.get().0, 2); // newly assigned
+    }
+}

src/context/offline.rs

@@ -43,7 +43,8 @@ impl OfflineAudioContext {
         // unbounded is fine because it does not need to be realtime safe
         let (sender, receiver) = crossbeam_channel::unbounded();
 
-        let graph = crate::render::graph::Graph::new();
+        let (node_id_producer, node_id_consumer) = llq::Queue::new().split();
+        let graph = crate::render::graph::Graph::new(node_id_producer);
         let message = crate::message::ControlMessage::Startup { graph };
         sender.send(message).unwrap();
 
@@ -67,6 +68,7 @@ impl OfflineAudioContext {
             sender,
             None,
             true,
+            node_id_consumer,
         );
 
         Self {

src/context/online.rs

@@ -173,7 +173,8 @@ impl AudioContext {
             event_recv,
         } = control_thread_init;
 
-        let graph = Graph::new();
+        let (node_id_producer, node_id_consumer) = llq::Queue::new().split();
+        let graph = Graph::new(node_id_producer);
         let message = ControlMessage::Startup { graph };
         ctrl_msg_send.send(message).unwrap();
 
@@ -184,6 +185,7 @@ impl AudioContext {
             ctrl_msg_send,
             Some((event_send, event_recv)),
             false,
+            node_id_consumer,
         );
         base.set_state(AudioContextState::Running);
 

src/media_devices/mod.rs

@@ -4,7 +4,7 @@
 //!
 //! <https://developer.mozilla.org/en-US/docs/Web/API/MediaDevices>
 
-use rustc_hash::FxHasher;
+use std::collections::hash_map::DefaultHasher;
 use std::hash::{Hash, Hasher};
 
 use crate::context::{AudioContextLatencyCategory, AudioContextOptions};
@@ -54,7 +54,7 @@ impl DeviceId {
             index,
         };
 
-        let mut hasher = FxHasher::default();
+        let mut hasher = DefaultHasher::new();
         device_info.hash(&mut hasher);
         format!("{}", hasher.finish())
     }

src/message.rs

@@ -14,6 +14,7 @@ pub(crate) enum ControlMessage {
     /// Register a new node in the audio graph
     RegisterNode {
         id: AudioNodeId,
+        reclaim_id: llq::Node<AudioNodeId>,
         node: Box<dyn AudioProcessor>,
         inputs: usize,
         outputs: usize,