Take advantage of the separation between mutable and immutable subsystem calls

[iljakuklic]

We have two methods to call functions on subsystems: Subsystem::call and Subsystem::call_mut. These expose the subsystem internal state as an immutable and mutable reference, respectively. Currently, the call method just delegates to the call_mut method. A separate implementation for call could be provided that takes the advantage of the immutability to execute multiple calls simultaneously.

[Ivansete]

Good afternoon @iljakuklic,

While checking this issue I don't fully understand why "call_mut" is actually needed. I have the impression that all the Actions, and their CallResult responses, could be done properly throughout the UnboundedChannel only using the immutable methods (System::call) but I might be missing some details.

May kindly share a couple of use cases where "call_mut" and "call" is needed and why please ? That will help me to understand it better.

Thanks in advance

[iljakuklic]

Good afternoon @iljakuklic,

While checking this issue I don't fully understand why "call_mut" is actually needed. I have the impression that all the Actions, and their CallResult responses, could be done properly throughout the UnboundedChannel only using the immutable methods (System::call) but I might be missing some details.

In call_mut you are given a &mut reference to the subsystem object whereas call only gives you access to & reference to the subsystem object. The former is necessary if you want to apply any modifying operation (e.g. call a method that is defined on &mut self) on the subsystem.

The idea behind this issue is that when using shared references (&), multiple calls could be potentially dispatched and processed simultaneously in multiple threads/tasks. The difficulty is that more synchronisation will be needed to ensure that a call_mut is not processed simultaneously with another call_mut or with a call. It's hard to tell whether this would be a win overall, given the need for extra synchronisation, the fact that new tasks would have to be spawned, etc.

May kindly share a couple of use cases where "call_mut" and "call" is needed and why please ? That will help me to understand it better.

You can have a look in subsystem/tests/passive.rs. It contains a simple example. It's rather artificial but it illustrates the high level concepts reasonably well.

Thanks in advance

Hope this helps.

[Ivansete]

Morning @iljakuklic and thanks for the reply.

I understand it but I have the impression that we could never achieve a parallel execution as there is a tokio::unbounded mpsc channel, meaning that all messages are enqueued and only processed by one single receiver, i.e. the messages can only be processed on after the other, sequentially.

I agree that there should be the synchronization mechanism that you mentioned in your previous comment.

In order to achieve parallel execution I think there might be a pool of threads that share a reference to the Subsystem, wrapped in a RwLock, and we should use a mpmc approach with either flume or async_channel. That thread-subsystem pool will act as a consumers pool.

Feel free to correct me if needed or disagree.

[iljakuklic]

Morning @iljakuklic and thanks for the reply.

I understand it but I have the impression that we could never achieve a parallel execution as there is a tokio::unbounded mpsc channel, meaning that all messages are enqueued and only processed by one single receiver, i.e. the messages can only be processed on after the other, sequentially.

Yes, the implementation would have to change substantially to do it.

I agree that there should be the synchronization mechanism that you mentioned in your previous comment.

In order to achieve parallel execution I think there might be a pool of threads that share a reference to the Subsystem, wrapped in a RwLock, and we should use a mpmc approach with either flume or async_channel. That thread-subsystem pool will act as a consumers pool.

Thank you for researching this.

Feel free to correct me if needed or disagree.

To be perfectly honest, I would consider tackling this issue at this point a premature optimisation:

• We do not know the current approach poses a major issue, even if it ends up being slower than what is being proposed. There may be other more important bottlenecks in the system, e.g. to do with IO.
• A reasonably comprehensive set of benchmarks should be devised first so we know whether this is an overall win or not. The synchronisation overhead may end up being quite substantial.
• There is some merit in the simplicity of the current "sequential" approach. It's easier to understand and to analyse potential issues.

[Ivansete]

Thanks for the reply @iljakuklic.

I agree with your points and for now this represents a premature optimization.