perf(rt): eliminate permit_alloc sites on the RT audio path (#239) (#254)

Author: OpenSauce

rustortion-core/Cargo.toml

@@ -16,12 +16,14 @@ anyhow = "1.0"
 rustfft = "6.4"
 realfft = "3.5"
 arc-swap = "1.8"
-assert_no_alloc = { version = "1.1", features = ["warn_debug"] }
 nam-rs = "0.2.0"
 
 [dev-dependencies]
 criterion = { version = "0.8", features = ["html_reports"] }
 tempfile = "3.24"
+# Test-only: RT-path allocation auditing (tests/no_alloc.rs). `warn_debug`
+# makes it count violations instead of aborting the test binary.
+assert_no_alloc = { version = "1.1", features = ["warn_debug"] }
 
 [[bench]]
 name = "impulse_responses"

rustortion-core/benches/common/mod.rs

@@ -24,7 +24,7 @@ pub fn create_test_cabinet(ir_length: usize, sample_rate: usize) -> IrCabinet {
 
     let mut convolver = Convolver::new_fir(max_ir_samples);
     convolver.set_ir(&ir_samples).unwrap();
-    cabinet.swap_convolver(convolver);
+    cabinet.set_convolver(convolver);
 
     cabinet
 }

rustortion-core/src/amp/chain.rs

@@ -5,16 +5,37 @@ struct BypassableStage {
     bypassed: bool,
 }
 
+/// Stage capacity reserved up front, and the hard cap on chain length.
+///
+/// `insert_stage` (the RT `AddStage` path) never grows the backing `Vec` past
+/// its reserved capacity, so it never reallocates on the RT thread; the UI
+/// enforces the same number as the maximum stage count. ~24 bytes per slot, so
+/// ~1.5 KB reserved.
+pub const DEFAULT_CHAIN_CAPACITY: usize = 64;
+
 // AmplifierChain holds a sequence of processing stages.
-#[derive(Default)]
 pub struct AmplifierChain {
     stages: Vec<BypassableStage>,
 }
 
+impl Default for AmplifierChain {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 impl AmplifierChain {
     #[must_use]
-    pub const fn new() -> Self {
-        Self { stages: Vec::new() }
+    pub fn new() -> Self {
+        Self::with_capacity(DEFAULT_CHAIN_CAPACITY)
+    }
+
+    /// Create a chain that can hold `capacity` stages before reallocating.
+    #[must_use]
+    pub fn with_capacity(capacity: usize) -> Self {
+        Self {
+            stages: Vec::with_capacity(capacity),
+        }
     }
 
     pub fn add_stage(&mut self, stage: Box<dyn Stage>) {
@@ -62,8 +83,18 @@ impl AmplifierChain {
         self.stages.get(idx).map(|s| s.inner.get_parameter(name))
     }
 
-    /// Insert a stage at the given index.
-    pub fn insert_stage(&mut self, idx: usize, stage: Box<dyn Stage>) {
+    /// Insert a stage at the given index **without reallocating**.
+    ///
+    /// This runs on the RT thread (via `AddStage`). If the chain is already at
+    /// its reserved capacity, the stage is **returned** rather than inserted, so
+    /// the caller can dispose of it off the RT thread (growing the `Vec` here
+    /// would allocate, and dropping the rejected box here would free, on the
+    /// audio thread). Returns `None` when the stage was inserted.
+    #[must_use]
+    pub fn insert_stage(&mut self, idx: usize, stage: Box<dyn Stage>) -> Option<Box<dyn Stage>> {
+        if self.stages.len() == self.stages.capacity() {
+            return Some(stage);
+        }
         let idx = idx.min(self.stages.len());
         self.stages.insert(
             idx,
@@ -72,6 +103,7 @@ impl AmplifierChain {
                 bypassed: false,
             },
         );
+        None
     }
 
     /// Remove and return the stage at the given index.
@@ -144,7 +176,7 @@ mod tests {
         let mut chain = AmplifierChain::new();
         chain.add_stage(make_level(1.0));
         chain.add_stage(make_level(1.0));
-        chain.insert_stage(1, make_level(0.5));
+        assert!(chain.insert_stage(1, make_level(0.5)).is_none());
         let out = chain.process(1.0);
         assert!((out - 0.5).abs() < 1e-6);
     }
@@ -239,7 +271,7 @@ mod tests {
         let mut chain = AmplifierChain::new();
         chain.add_stage(make_level(1.0));
         chain.set_bypassed(0, true);
-        chain.insert_stage(0, make_level(0.5)); // insert before bypassed
+        assert!(chain.insert_stage(0, make_level(0.5)).is_none()); // insert before bypassed
         // idx 0 = new (active, 0.5x), idx 1 = old (bypassed, 1.0x)
         let out = chain.process(1.0);
         assert!(

rustortion-core/src/audio/engine.rs

@@ -1,5 +1,4 @@
 use anyhow::Result;
-use assert_no_alloc::permit_alloc;
 use crossbeam::channel::{Receiver, Sender, bounded};
 use log::{debug, error};
 
@@ -17,7 +16,10 @@ use crate::tuner::Tuner;
 
 pub struct PreparedIr {
     pub name: String,
-    pub convolver: Convolver,
+    /// Boxed so it can be swapped into the cabinet on the RT thread without
+    /// reallocating, and so the whole `PreparedIr` (old convolver + name) can
+    /// be retired off the RT thread in one piece.
+    pub convolver: Box<Convolver>,
 }
 
 pub enum EngineMessage {
@@ -35,7 +37,9 @@ pub enum EngineMessage {
     SetIrBypass(bool),
     SetIrGain(f32),
     SetTunerEnabled(bool),
-    SetPitchShift(i32),
+    /// Carries a fully-constructed pitch shifter (built off the RT thread), or
+    /// `None` to disable pitch shifting (the `0` semitones bypass case).
+    SetPitchShift(Option<Box<PitchShifter>>),
     SetStageBypassed(usize, bool),
     SetSamplers(Box<Samplers>),
 }
@@ -49,12 +53,14 @@ pub struct Engine {
     engine_receiver: Receiver<EngineMessage>,
     /// Handle for sending arbitrary objects off the RT thread for deallocation.
     rt_drop: RtDropHandle,
-    samplers: Samplers,
+    /// Boxed so swapping samplers (sample-rate / buffer changes) on the RT
+    /// thread exchanges pointers and retires the old box directly.
+    samplers: Box<Samplers>,
     tuner: Option<Tuner>,
     recorder: Option<Recorder>,
     peak_meter: Option<PeakMeter>,
     metronome: Option<Metronome>,
-    pitch_shifter: Option<PitchShifter>,
+    pitch_shifter: Option<Box<PitchShifter>>,
     input_highpass: Option<Box<dyn Stage>>,
     input_lowpass: Option<Box<dyn Stage>>,
     /// When true, skip tuner, peak meter, recorder, and metronome processing.
@@ -83,7 +89,7 @@ impl Engine {
                 ir_cabinet,
                 engine_receiver,
                 rt_drop,
-                samplers,
+                samplers: Box::new(samplers),
                 tuner: Some(tuner),
                 recorder: None,
                 peak_meter: Some(peak_meter),
@@ -115,7 +121,7 @@ impl Engine {
             ir_cabinet,
             engine_receiver,
             rt_drop: rt_drop_handle,
-            samplers,
+            samplers: Box::new(samplers),
             tuner: None,
             recorder: None,
             peak_meter: None,
@@ -162,27 +168,21 @@ impl Engine {
         }
 
         if let Some(ref mut shifter) = self.pitch_shifter {
-            // FIXME(no_alloc): PitchShifter::process_block uses Vec scratch
-            // buffers internally — see audio/pitch_shifter.rs.
-            permit_alloc(|| shifter.process_block(output));
+            shifter.process_block(output);
         }
 
         if let Some(ref mut cab) = self.ir_cabinet {
             cab.process_block(output);
         }
 
         if let Some(ref mut peak_meter) = self.peak_meter {
-            // FIXME(no_alloc): PeakMeter::process does Arc::new(PeakMeterInfo)
-            // every block — see audio/peak_meter.rs:62.
-            permit_alloc(|| peak_meter.process(output));
+            peak_meter.process(output);
         }
 
         if !self.lightweight
             && let Some(recorder) = self.recorder.as_mut()
         {
-            // FIXME(no_alloc): Recorder::record_block does Vec::with_capacity
-            // per block — see audio/recorder.rs:47.
-            permit_alloc(|| recorder.record_block(output))?;
+            recorder.record_block(output);
         }
 
         Ok(())
@@ -263,8 +263,14 @@ impl Engine {
                     }
                 }
                 EngineMessage::AddStage(idx, stage) => {
-                    self.chain.insert_stage(idx, stage);
-                    debug!("Added stage at index {idx}");
+                    if let Some(rejected) = self.chain.insert_stage(idx, stage) {
+                        // Chain is at its reserved capacity. Retire the rejected
+                        // stage off the RT thread rather than dropping (freeing)
+                        // it here. The UI caps stage count, so this is a backstop.
+                        self.rt_drop.retire(rejected);
+                    } else {
+                        debug!("Added stage at index {idx}");
+                    }
                 }
                 EngineMessage::RemoveStage(idx) => {
                     if let Some(old) = self.chain.remove_stage(idx) {
@@ -286,16 +292,26 @@ impl Engine {
                     }
                 }
                 EngineMessage::SetInputFilters(hp, lp) => {
-                    self.input_highpass = hp;
-                    self.input_lowpass = lp;
+                    // Retire the previous filters off the RT thread instead of
+                    // dropping them here on direct assignment.
+                    if let Some(old) = std::mem::replace(&mut self.input_highpass, hp) {
+                        self.rt_drop.retire(old);
+                    }
+                    if let Some(old) = std::mem::replace(&mut self.input_lowpass, lp) {
+                        self.rt_drop.retire(old);
+                    }
                     debug!("Updated input filters");
                 }
-                EngineMessage::SwapIrConvolver(prepared) => {
+                EngineMessage::SwapIrConvolver(mut prepared) => {
                     if let Some(ref mut cab) = self.ir_cabinet {
                         debug!("IR convolver swapped: {}", prepared.name);
-                        let old = cab.swap_convolver(prepared.convolver);
-                        permit_alloc(|| self.rt_drop.retire(Box::new(old)));
+                        // Swap the new convolver in; `prepared` is left holding
+                        // the old convolver. Retire the whole `PreparedIr` (old
+                        // convolver + name `String`) off the RT thread so
+                        // nothing deallocates here.
+                        cab.swap_convolver(&mut prepared.convolver);
                     }
+                    self.rt_drop.retire(prepared);
                 }
                 EngineMessage::ClearIr => {
                     if let Some(ref mut cab) = self.ir_cabinet {
@@ -326,12 +342,12 @@ impl Engine {
                 EngineMessage::StopRecording => {
                     self.handle_stop_recording();
                 }
-                EngineMessage::SetPitchShift(semitones) => {
-                    self.handle_pitch_shift(semitones);
+                EngineMessage::SetPitchShift(shifter) => {
+                    self.handle_pitch_shift(shifter);
                 }
                 EngineMessage::SetSamplers(new_samplers) => {
-                    let old = std::mem::replace(&mut self.samplers, *new_samplers);
-                    permit_alloc(|| self.rt_drop.retire(Box::new(old)));
+                    let old = std::mem::replace(&mut self.samplers, new_samplers);
+                    self.rt_drop.retire(old);
                     debug!("Samplers swapped");
                 }
             }
@@ -364,19 +380,15 @@ impl Engine {
         self.recorder = None;
     }
 
-    fn handle_pitch_shift(&mut self, semitones: i32) {
-        if semitones == 0 {
-            self.pitch_shifter = None;
-            debug!("Pitch shift disabled (bypass)");
-        } else if let Some(ref mut shifter) = self.pitch_shifter {
-            shifter.set_semitones(semitones as f32);
-            debug!("Pitch shift set to {semitones} semitones");
-        } else {
-            // FIXME(no_alloc): PitchShifter::new allocates FFT scratch buffers
-            // — see audio/pitch_shifter.rs.
-            self.pitch_shifter = Some(permit_alloc(|| PitchShifter::new(semitones as f32)));
-            debug!("Pitch shift set to {semitones} semitones");
+    fn handle_pitch_shift(&mut self, shifter: Option<Box<PitchShifter>>) {
+        // The shifter (if any) is constructed off the RT thread in
+        // `EngineHandle::set_pitch_shift`; here we just swap it in and retire
+        // the previous one off the RT thread.
+        let old = std::mem::replace(&mut self.pitch_shifter, shifter);
+        if let Some(old) = old {
+            self.rt_drop.retire(old);
         }
+        debug!("Pitch shifter updated");
     }
 }
 
@@ -448,8 +460,14 @@ impl EngineHandle {
     }
 
     pub fn set_pitch_shift(&self, semitones: i32) {
-        let update = EngineMessage::SetPitchShift(semitones);
-        self.send(update);
+        // Construct the pitch shifter here (GUI thread) so the RT thread never
+        // allocates its FFT plans / scratch buffers. `0` semitones == bypass.
+        let shifter = if semitones == 0 {
+            None
+        } else {
+            Some(Box::new(PitchShifter::new(semitones as f32)))
+        };
+        self.send(EngineMessage::SetPitchShift(shifter));
     }
 
     pub fn set_stage_bypassed(&self, idx: usize, bypassed: bool) {
@@ -461,8 +479,13 @@ impl EngineHandle {
         self.send(update);
     }
 
-    pub fn start_recording(&self, sample_rate: usize, output_dir: &str) -> Result<()> {
-        let recorder = Recorder::new(sample_rate as u32, output_dir)?;
+    pub fn start_recording(
+        &self,
+        sample_rate: usize,
+        output_dir: &str,
+        max_block_samples: usize,
+    ) -> Result<()> {
+        let recorder = Recorder::new(sample_rate as u32, output_dir, max_block_samples)?;
 
         let update = EngineMessage::StartRecording(recorder);
         self.send(update);