locka99 / opcua

A client and server implementation of the OPC UA specification written in Rust
Mozilla Public License 2.0
480 stars 129 forks source link

non-blocking variable writes #176

Open lovasoa opened 2 years ago

lovasoa commented 2 years ago

Looking at the code base, it looks like all the datastructures are full of Arc<std::sync::RwLock<T>>. Notably, the address space is a Arc<std::sync::RwLock<HashMap<NodeId, Node>>>. Would you be open to a big pull request that would replace that with a concurrent hashmap, to avoid blocking the entire server on every variable write ?

This would probably be quite a large PR, so I would like to discuss it here before starting the work. If you don't have the time to review a large pr, that's ok, I can fork this repo and publish it under a different name.

locka99 commented 2 years ago

I can think of a few simple ways to fix this, depending on your needs:

  1. Call write() service with more than one variable to write in a single call. The lock is per service call, so many variables can be written at once
  2. Change lock granularity in write() call, so address space is locked in the loop and then unlocked so it is not blocking any other function in the server.
  3. Change the node_map to be itself arc rwlock so only a read lock is needed on address space, temporarily to clone the nodemap reference and then independently use it.
  4. Perhaps change node_map so each value is reference counted although this could be expensive (thousands of nodes in address space), that way a mut ref isn't required at all on the node_map or the address so both can be read locked and only temporarily.
lovasoa commented 2 years ago

What I am proposing is to use a concurrent hashmap, so that none of these workarounds has to be implemented.

There are multiple off the shelf concurrent hashmaps available in rust. Here is a comparison: https://github.com/xacrimon/conc-map-bench

dashmap is a popular choice: https://github.com/xacrimon/dashmap