how to design a single write and multiple read when the protected resource need to be return by getter as a shared_ptr in modern c++?

user_1569058 0
Viewed 294

I have one writer thread write to a map in an object, and many reader object each in their own thread to perform read only on the map, my currrent design is like below,

class MyClass {
private:
    shared_ptr<map<int, int>> my_map;
    mutable shared_mutex mu;
public:
    void update_or_add(pair<int, int> entry);
    shared_lock<shared_mutex> get_read_lock();
    shared_ptr<const map<int, int>> get_map();
}


void MyClass::update_or_add(pair<int, int> entry) {
    lock_guard<shared_mutex> locker(mu);
    // do update on my_map
}

shared_lock<shared_mutex> MyClass::get_read_lock() {
    shared_lock<shared_mutex> locker(mu);
    return locker;

shared_ptr<const map<int, int>> MyClass::get_map() {
    return my_map;
}

here is how I may use it

//writer
//thread-1
MyClass obj;
while(true) 
obj.update_or_add(entry);

//readers
//thread-2
shared_lock<shared_mutex> locker(obj.get_read_lock());
auto mymap = obj.get_map();
//some computation on mymap
....
//thread-n
shared_lock<shared_mutex> locker(obj.get_read_lock());
auto mymap = obj.get_map();
//some computation on mymap

I have two questions regard the above,

  1. is this the right way to use shared_lock and shared_mutex?
  2. because I want to achieve concurrency in read, so I have returned the map's pointer and passed it to other object to do the computation, is there any other better design?

Thanks.

c++14 c++17 mutex concurrency c++
1 Answers

Here is a really simple shared resource:

template<class T>
struct shared_threadsafe {
  template<class F>
  auto read(F&& f)const{
    auto l = lock(); // shared lock
    return f(t); // const access
  }
  template<class F>
  auto write(F&& f){
    auto l = lock(); // exclusive lock
    return f(t); // non-const access
  }
  // construct from a T
  explicit shared_threadsafe( T in ):t(std::forward<T>(in)){}
  // default construct
  shared_threadsafe()=default;
  shared_threadsafe(shared_threadsafe const&)=delete; // it can be written, but don't
  shared_threadsafe& operator=(shared_threadsafe const&)=delete; // it can be written, but don't
  ~shared_threadsafe()=default;
private:
  mutable std::shared_mutex m;
  T t;
  auto lock() const { return std::shared_lock{m}; }
  auto lock() { return std::unique_lock{m}; }
}

it uses c++17 to be really brief. Apologies for typos.

You have a:

shared_threadsafe<std::map<int,int>> bob;

Writer thread:

bob.write([&](auto& bob){ bob.insert( p ); });

reader thread:

bob.read([&](auto& bob){
  // cos=de that reads from the map `bob`
});

if reader threads have const references or pointers, they only have read access.

In practice, I'd think up a few operations, have the shared threadsafe be private in some class, and expose only those operations.

Generally exposing full read/write locks is an indication your code isn't high level enough yet. Lock based concurrency does not compose nor does it scale well. When you expose the ability to run arbitrary code within a lock, you encourage composition, which is a disaster.

My shared threadsafe is an attempted compromise, because at least the lock code is written once and scope based. I hide it because it still does not compose.

Depending on what your read/write operations are and what the cost of delays is, moving to a copy on contention version might also be wise. Under this plan, the readers get smart pointers to the table they have full const access to. Writers do a copy in write (cow) if there is contention, and otherwise do fast updates. The copy on write result is only visible to readers who come along after the write occurs.

A hand written (non-std) map can do partial cow update, sharing parts of the table that are unchanged between old and new reads.

But at this point, you start considering a DB. Which maybe you should start with anyhow.

user_17746670
Related