Feature request: allow AudioNodes to be !Send and !Sync

[Smona]

Our use-case for fundsp is a single-threaded audio engine which receives commands over a message channel. We have no need to send or share AudioNodes between threads, and we've also found that atomic operations and other synchronization primitives expectedly reduce performance, particularly in WASM.

We've run up against this limitation in our efforts to optimize performance, and it prevents us from using faster interior mutability types like Cell in uses cases that would otherwise work, and forces us to resort to unsafe in ways that rely on implicit assumptions of our codebase (preventing us from ever providing that code in a library in the future). Since it doesn't seem like any functionality of the trait inherently requires these, it would be great if this was more of an opt-in thing, and it would unlock a significant increase in performance ceiling for applications leveraging fundsp.

[Smona]

It seems like this could be implemented in two ways relatively straightforwardly:

• a default-enabled sync feature which enables Send + Sync bounds on AudioNode & AudioUnit
• Wrapper or alternative types for Sync vs !Sync AudioNodes & AudioUnits.

Do you have a preference on how something like this should be supported? I'd be happy to take a shot at it.

[Smona]

I was able to get a basic POC working with tests passing here, without any new features, by simply introducing a SharedUnit wrapper trait around AudioUnit and using that in any types that support backends!

It probably needs a little more work akin to what I did in FeedbackUnit: making units that wrap other units support both shared and unshared AudioUnits via a generic. But it seems promising!

[SamiPerttu]

Hi, yes I'd like to support this. I was thinking about the best way to do it. I think the feature approach could be fairly clean and non-intrusive. Currently, Net and Sequencer don't have separate frontends, which is not a great design. So that should also be done to accomplish this. Fortunately it shouldn't be a lot of work. What do you think?

[Smona]

Heya, glad to hear it! We've used Net but not Sequencer. Our usage of Net is similar to what I described, where we're using it in a single-threaded context. So it would be great if it supported !Send+!Sync units as well (a shortcoming of the POC i linked).

I don't have enough understanding of those structs to strongly favor a particular solution around frontends, but i could imagine the Net potentially having a generic parameter for the types of units contained. We've actually gone around using Box<dyn AudioUnit> in favor of a NodeType(<concrete unit>) trait enum in other places, accepting increased memory usage so we can avoid the slowdown of dynamic dispatch + pointer indirection. A generic would also support this pattern inside Net, which would be great for performance. Then any frontend/backend functionality that requires the contained units to be Send + Sync could potentially be gated behind a trait bound on that generic, e.g. impl<T: AudioUnit + Send + Sync + Clone> Net<T>?

I will say I think it would be great for users that want to use a frontend to a Net from another thread if that interface didn't require the nodes themselves to be Send + Sync. FWIW, in our project we basically have a trait interface to our graph nodes with a set(node_id, param_id, value) method, and in-between process() calls the thread which owns the AudioUnits consumes "set" commands from a channel to set a thread-local shared type embedded into its inner AudioUnit to that value. I could see Sequencer and Net having a similar way to map messages from another thread to thread-local updates of interior-mutable types, keeping any atomic accesses out of the Net's process/tick methods. But not sure if that jives with how you intended backends for those types to be used.

I did look into using a feature flag first, and I found some potential downsides. This forum post clarified things: not only would this be slightly working against how feature flags are meant to be used, you also can't cfg-gate supertraits. It would be really undesirable to copy the entire trait definitions for AudioNode and AudioUnit, and introducing an intermediate trait felt messy. Besides, I can see it being very likely that some projects would want to mix Sync and !Sync units in the same crate, which the feature flag approach would complicate a lot. so at this point the addition of generics to any structs which wrap AudioUnits, and Send + Sync gating for specific functionality that requires it, seems much more flexible and future-proof to me.

[SamiPerttu]

Armed with this perspective, we need to discard the feature idea. Let's use your traits solution. Let me know if you need anything.

[Smona]

Sweet, I think we're on the same page! I'm starting to work through those changes.

One question for you: there are some methods on Net which rely on the assumption that it will always hold Box<dyn AudioUnit>. Some, like Net::scalar, are constructors so they can work with the generic (they would just always return Net<Box<dyn AudioUnit>>. But for the methods like fade_in(), I don't think we can really support them with other generic parameters.

I'm thinking it probably makes sense to move these specific functions to a separate impl Net<Box<dyn AudioUnit>>. This way we can avoid breaking the methods for existing users, and anyone who wants to use a different inner type will just have to implement their own version of them. Does that sound good to you?

Alternatively, the methods could just be removed in favor of a more minimal maintenance surface. That feels like it would be an unnecessary loss though.

[Smona]

I've managed to make Net generic over AudioUnits here! It's indeed pretty involved, but frankly not too bad. It should be relatively backwards compatible, keeping all existing functionality implemented for the Box<dyn AudioUnit> case. The only Net-specific breaking change I'm aware of is that type annotations will now be needed when constructing Nets, a change which the BoxedNet alias aims to alleviate by making the fix just s/Net/BoxedNet/g.

Let me know what you think! I'm particularly interested in how you feel about the NetUnit abstraction I opted for to handle the case described above. In the meantime, I'll start working on cleaning up the change (especially docs), and testing it against our crate. I think the changes included in the branch might already cover our desired use case, but also let me know if there are other changes you want bundled with this one (e.g. similar parameterization on Sequencer)